iop/agent-task/cli_usage_status_command/plan_1.log

256 lines
12 KiB
Text

<!-- task=cli_usage_status_command plan=1 tag=REVIEW_API -->
# Review Follow-up Plan
## 이 파일을 읽는 구현 에이전트에게
아래 체크리스트를 순서대로 완료하고, 각 항목의 중간 검증과 최종 검증을 실제로 실행하세요. 구현이 끝나면 `agent-task/cli_usage_status_command/CODE_REVIEW.md`의 모든 섹션을 실제 구현 내용과 명령 출력으로 채우세요. `CODE_REVIEW.md`의 `이 파일을 읽는 리뷰 에이전트에게` 섹션에 있는 아카이브 지시(`*.log`로 이름 변경, `complete.log` 작성)는 구현 에이전트가 수행하면 안 되며, 리뷰 스킬 전용입니다.
## 배경
1차 구현은 node command 계약과 edge console `/status` 흐름을 추가했지만, 리뷰에서 기본 `configs/edge.yaml`이 실제 codex 실행을 `bash` mock으로 바꿔 기능을 깨는 문제가 확인되었습니다. 또한 status 테스트가 외부 binary 실행 실패에 의존해 계획한 agent/profile 선택 검증을 수행하지 못하고, console timeout 보정과 Go formatting 문제가 남았습니다. 이번 follow-up은 기능 동작을 깨는 설정 변경을 되돌리고, 테스트 신뢰도와 기본 품질 검사를 회복하는 데 집중합니다.
## 의존 관계 및 구현 순서
1. `REVIEW_API-1`에서 예시 설정을 실제 codex profile로 복원한다.
2. `REVIEW_API-2`에서 status checker/factory 테스트를 외부 실행 의존 없이 바꾼다.
3. `REVIEW_API-3`에서 `/status` timeout 보정 버그를 고친다.
4. `REVIEW_API-4`에서 formatting과 whitespace를 정리한다.
### [REVIEW_API-1] `configs/edge.yaml` codex profile 복원
#### 문제
`configs/edge.yaml:59-70`의 `codex` profile이 실제 `codex exec --json` 설정에서 `bash` persistent loop로 바뀌었습니다. 현재 `console.agent`도 `configs/edge.yaml:16`에서 `codex`이므로 `/status`는 `agent == "codex"` 분기로 `CodexChecker("bash")`를 만들고 `bash --no-alt-screen`을 실행하게 됩니다. 이 변경은 계획 범위를 벗어나며, 기본 실행과 status 조회를 모두 실제 codex 경로에서 이탈시킵니다.
Before (`configs/edge.yaml:59`):
```yaml
codex:
command: "bash"
args:
- "-lc"
- |
while IFS= read -r line; do
printf 'codex-shell> %s\n' "$line"
done
env: []
persistent: true
terminal: false
```
#### 해결 방법
`codex` profile을 기존 `codex exec --dangerously-bypass-approvals-and-sandbox --color never --skip-git-repo-check --json` 형태로 복원하고 `output_format: "codex-json"`도 되돌립니다. 만약 수동 smoke test용 shell profile이 필요하면 `codex`를 바꾸지 말고 별도 profile 이름으로 추가하며, 기본 `console.agent` 변경이 필요하면 그 이유를 `CODE_REVIEW.md`에 기록합니다.
After:
```yaml
codex:
command: "codex"
args:
- "exec"
- "--dangerously-bypass-approvals-and-sandbox"
- "--color"
- "never"
- "--skip-git-repo-check"
- "--json"
env: []
persistent: true
terminal: false
output_format: "codex-json"
```
#### 수정 파일 및 체크리스트
- [ ] `configs/edge.yaml` - `codex` profile command/args/output_format 복원
- [ ] `configs/edge.yaml` - `console.agent` 값을 유지하거나 변경 이유를 명시
- [ ] `apps/edge/internal/transport/server_test.go` 또는 `packages/config/config_test.go` - 예시 config의 codex profile이 `command: codex`, `output_format: codex-json`를 유지하는지 필요한 경우 검증
#### 테스트 작성
테스트를 작성합니다. 최소한 `config.LoadEdge("configs/edge.yaml")` 또는 payload 생성 경로를 통해 `codex` profile이 `Command == "codex"`와 `OutputFormat == "codex-json"`을 유지하는지 검증하세요.
#### 중간 검증
```bash
go test ./packages/config/... ./apps/edge/internal/transport/...
```
기대 결과: 예시 config 로딩/transport payload 테스트가 통과하고, codex profile이 실제 codex command로 유지됩니다.
### [REVIEW_API-2] status command 테스트를 fake checker/factory 기반으로 변경
#### 문제
`apps/node/internal/adapters/cli/cli_test.go:15-48`은 `HandleCommand`가 선택한 agent/profile을 쓰는지 확인하기 위해 실제 `/tmp/non-existent-codex` 실행 실패를 기대합니다. `apps/node/internal/adapters/cli/status/status_test.go:12-17`도 `/tmp/mycodex` 실행 실패 문자열에 의존합니다. 이 방식은 계획의 "fake checker/factory로 현재 agent의 profile command가 전달되는지 검증" 요구를 만족하지 못하고, OS별 exec error 문구나 파일 존재 여부에 따라 흔들릴 수 있습니다.
Before (`apps/node/internal/adapters/cli/cli_test.go:37`):
```go
_, err = c.HandleCommand(context.Background(), runtime.CommandRequest{
Type: runtime.CommandTypeUsageStatus,
Model: "codex",
})
// Since we mock nothing here, the underlying Codex checker will attempt to run `codex`
// and will fail. We just care that it doesn't return "unknown agent".
if err == nil {
t.Errorf("expected error because codex binary is likely not present in tests, but got none")
}
```
#### 해결 방법
status package에 실행을 동반하지 않는 factory seam을 작게 둡니다. 예를 들어 `NewChecker(agent string, profile config.CLIProfileConf) (Checker, error)` 같은 순수 factory를 분리하고, `CheckUsage`는 그 factory를 호출한 뒤 `Check(ctx)`만 실행하게 합니다. `CLI.HandleCommand` 테스트는 package-level 테스트 hook 또는 internal fake factory를 통해 fake checker를 주입해 `req.Model == "codex"`일 때 `profile.Command`가 그대로 전달되는지 assertion합니다. 외부 binary 실행 실패를 성공 조건으로 삼지 않습니다.
After:
```go
checker, err := NewChecker(agent, profile)
if err != nil {
return nil, err
}
return checker.Check(ctx)
```
#### 수정 파일 및 체크리스트
- [ ] `apps/node/internal/adapters/cli/status/status.go` - checker 선택과 checker 실행을 분리
- [ ] `apps/node/internal/adapters/cli/status/status_test.go` - factory가 `agent=codex`, `profile.Command=/tmp/mycodex`에서 codex checker를 선택하고 command를 보존하는지 외부 실행 없이 검증
- [ ] `apps/node/internal/adapters/cli/cli.go` - 필요한 경우 테스트 가능한 status runner/factory 주입점 추가
- [ ] `apps/node/internal/adapters/cli/cli_test.go` - fake checker/factory로 `HandleCommand`가 현재 선택된 `Model`과 profile을 전달하는지 검증
- [ ] `rg -n "non-existent-codex|no such file or directory|likely not present|/tmp/mycodex" apps/node/internal/adapters/cli` 결과가 테스트 실패 의존을 남기지 않는지 확인
#### 테스트 작성
테스트를 수정합니다.
- `apps/node/internal/adapters/cli/status/status_test.go` - `TestNewChecker_CodexUsesProfileCommand`: checker 생성 결과가 Codex checker이고 command path가 보존되는지 검증
- `apps/node/internal/adapters/cli/cli_test.go` - `TestCLIHandleCommandUsageStatusUsesSelectedAgent`: fake checker가 받은 agent, command, request id, session id를 assertion
- `apps/node/internal/adapters/cli/cli_test.go` - unknown agent와 unsupported command error는 기존처럼 유지
#### 중간 검증
```bash
go test ./apps/node/internal/adapters/cli/status/... ./apps/node/internal/adapters/cli/...
```
기대 결과: CLI/status 테스트가 외부 codex binary 유무와 무관하게 통과합니다.
### [REVIEW_API-3] `/status` request timeout 보정값을 edge wait timeout에도 사용
#### 문제
`apps/edge/cmd/edge/console.go:253-265`의 `buildNodeCommandRequest`는 `timeoutSec <= 0`이면 request timeout을 30초로 보정합니다. 하지만 `apps/edge/cmd/edge/console.go:279`의 `sendConsoleStatus`는 원래 인자인 `timeoutSec+5`를 그대로 사용해 `timeoutSec == 0`이면 edge가 5초만 기다립니다. request는 30초로 node에 전달되는데 edge는 5초 후 timeout될 수 있습니다.
Before (`apps/edge/cmd/edge/console.go:275`):
```go
req, _ := buildNodeCommandRequest(adapter, agent, sessionID, timeoutSec)
timeout := time.Duration(timeoutSec+5) * time.Second
resp, err := toki.SendRequestTyped[*iop.NodeCommandRequest, *iop.NodeCommandResponse](
```
#### 해결 방법
`sendConsoleStatus`에서 보정된 `req.GetTimeoutSec()`를 기준으로 wait timeout을 계산합니다.
After:
```go
req, _ := buildNodeCommandRequest(adapter, agent, sessionID, timeoutSec)
timeout := time.Duration(req.GetTimeoutSec()+5) * time.Second
resp, err := toki.SendRequestTyped[*iop.NodeCommandRequest, *iop.NodeCommandResponse](
```
#### 수정 파일 및 체크리스트
- [ ] `apps/edge/cmd/edge/console.go` - wait timeout을 `req.GetTimeoutSec()+5` 기준으로 변경
- [ ] `apps/edge/cmd/edge/console_test.go` - `timeoutSec == 0`일 때 request timeout이 30으로 보정되는 테스트 유지/보강
- [ ] 가능하면 `sendConsoleStatus` timeout 계산을 작은 helper로 분리해 unit test에서 0 보정값을 직접 검증
#### 테스트 작성
테스트를 작성합니다. `statusWaitTimeout(req)` 같은 helper를 추가한다면 `TestStatusWaitTimeout_UsesNormalizedRequestTimeout`에서 `buildNodeCommandRequest(..., 0)` 결과가 35초 wait timeout으로 계산되는지 검증하세요.
#### 중간 검증
```bash
go test ./apps/edge/cmd/edge
```
기대 결과: console status timeout 보정 테스트가 통과합니다.
### [REVIEW_API-4] Go formatting과 whitespace check 통과
#### 문제
리뷰 중 `gofmt -l`이 아래 파일을 반환했습니다.
```text
apps/edge/cmd/edge/console.go
apps/edge/cmd/edge/console_test.go
apps/edge/internal/transport/server_test.go
apps/node/internal/adapters/cli/status/codex.go
apps/node/internal/adapters/cli/status/status.go
apps/node/internal/node/node_test.go
```
또한 `git diff --check`는 `apps/edge/cmd/edge/console.go:276`, `apps/edge/cmd/edge/console.go:299`, `apps/node/internal/node/node_test.go:451`의 whitespace 문제를 보고했습니다.
#### 해결 방법
변경된 Go 파일에 `gofmt`를 적용하고 `git diff --check`가 깨끗하게 통과하도록 trailing whitespace와 EOF blank line을 정리합니다.
#### 수정 파일 및 체크리스트
- [ ] `apps/edge/cmd/edge/console.go` - gofmt 및 trailing whitespace 제거
- [ ] `apps/edge/cmd/edge/console_test.go` - gofmt
- [ ] `apps/edge/internal/transport/server_test.go` - gofmt
- [ ] `apps/node/internal/adapters/cli/status/codex.go` - gofmt
- [ ] `apps/node/internal/adapters/cli/status/status.go` - gofmt
- [ ] `apps/node/internal/node/node_test.go` - gofmt 및 EOF blank line 정리
#### 테스트 작성
별도 테스트는 작성하지 않습니다. formatting/whitespace 문제는 `gofmt -l`과 `git diff --check` 명령으로 검증합니다.
#### 중간 검증
```bash
gofmt -l apps/edge/cmd/edge/console.go apps/edge/cmd/edge/console_test.go apps/edge/internal/transport/server_test.go apps/node/internal/adapters/cli/status/codex.go apps/node/internal/adapters/cli/status/status.go apps/node/internal/node/node_test.go
git diff --check
```
기대 결과: `gofmt -l` 출력이 비어 있고 `git diff --check`가 성공합니다.
## 수정 파일 요약
| 파일 | 항목 |
|------|------|
| `configs/edge.yaml` | REVIEW_API-1 |
| `apps/edge/internal/transport/server_test.go` | REVIEW_API-1, REVIEW_API-4 |
| `packages/config/config_test.go` | REVIEW_API-1 |
| `apps/node/internal/adapters/cli/status/status.go` | REVIEW_API-2, REVIEW_API-4 |
| `apps/node/internal/adapters/cli/status/status_test.go` | REVIEW_API-2 |
| `apps/node/internal/adapters/cli/cli.go` | REVIEW_API-2 |
| `apps/node/internal/adapters/cli/cli_test.go` | REVIEW_API-2 |
| `apps/edge/cmd/edge/console.go` | REVIEW_API-3, REVIEW_API-4 |
| `apps/edge/cmd/edge/console_test.go` | REVIEW_API-3, REVIEW_API-4 |
| `apps/node/internal/adapters/cli/status/codex.go` | REVIEW_API-4 |
| `apps/node/internal/node/node_test.go` | REVIEW_API-4 |
## 최종 검증
```bash
go test ./packages/config/... ./apps/edge/internal/transport/...
go test ./apps/node/internal/adapters/cli/status/... ./apps/node/internal/adapters/cli/...
go test ./apps/edge/cmd/edge
gofmt -l apps/edge/cmd/edge/console.go apps/edge/cmd/edge/console_test.go apps/edge/internal/transport/server_test.go apps/node/internal/adapters/cli/status/codex.go apps/node/internal/adapters/cli/status/status.go apps/node/internal/node/node_test.go
git diff --check
go test ./...
```
기대 결과: 모든 테스트가 통과하고, `gofmt -l` 출력이 비어 있으며, `git diff --check`가 성공합니다.