135 lines
6 KiB
Text
135 lines
6 KiB
Text
<!-- task=quality_improvements plan=0 tag=REFACTOR -->
|
|
|
|
# Code Review Reference - REFACTOR
|
|
|
|
## 개요
|
|
|
|
date=2026-04-24
|
|
task=quality_improvements, plan=0, tag=REFACTOR
|
|
|
|
## 이 파일을 읽는 리뷰 에이전트에게
|
|
|
|
각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요.
|
|
리뷰 완료 후 반드시 아래 순서로 아카이브하세요.
|
|
|
|
1. `CODE_REVIEW.md` → `code_review_0.log` (기존 code_review_*.log 수 = 0)
|
|
2. `PLAN.md` → `plan_0.log` (기존 plan_*.log 수 = 0)
|
|
3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성.
|
|
|
|
---
|
|
|
|
## 구현 항목별 완료 여부
|
|
|
|
| 항목 | 완료 여부 |
|
|
|------|---------|
|
|
| [REFACTOR-1] PROTOCOL.md 상태 표 업데이트 | [x] |
|
|
| [REFACTOR-2] Python `_shutdown` → `shutdown` 공개 메서드 전환 | [x] |
|
|
| [REFACTOR-3] Kotlin `nextNonce()` → `internal` 접근자 | [x] |
|
|
| [REFACTOR-4] VERSIONING.md nonce overflow 정책 추가 | [x] |
|
|
|
|
## 계획 대비 변경 사항
|
|
|
|
- Kotlin `nextNonce()`는 단순 `internal`로 변경 시 public inline 함수 `addRequestListenerTyped()`에서 접근할 수 없어 컴파일이 실패했다. 기존 파일의 패턴에 맞춰 `@PublishedApi internal`로 구현했다.
|
|
- 이 실행 환경에는 `python` 명령이 없어 Python 검증은 `python3 -m pytest`로 실행했다.
|
|
- Kotlin/Python 전체 테스트 중 로컬 소켓 바인딩이 필요한 케이스는 sandbox 권한 제한으로 실패하여 sandbox 밖에서 동일 검증을 재실행했다.
|
|
|
|
## 주요 설계 결정
|
|
|
|
- Python은 `_shutdown()`을 public `shutdown()`으로 이름만 전환하고 기존 종료 동작은 유지했다.
|
|
- Kotlin은 외부 public API 노출을 줄이되 public inline 헬퍼의 컴파일 가능성을 위해 `@PublishedApi internal`을 사용했다.
|
|
- `nonce` overflow 문서는 현재 프로토콜이 overflow를 정의하지 않으며 구현체도 복구 로직을 추가하지 않는다는 정책을 명시했다.
|
|
|
|
## 리뷰어를 위한 체크포인트
|
|
|
|
- PROTOCOL.md 표에서 TypeScript와 Python 두 행 모두 `Available`로 변경되었는지 확인
|
|
- `communicator.py`에서 `_shutdown` 이름이 모두 `shutdown`으로 변경됐는지 (정의 + self 호출 + `base_client.py` 외부 호출)
|
|
- `Communicator.kt:92`의 `nextNonce` 앞에 `internal` 키워드가 있는지, 동일 파일 내 top-level 헬퍼 함수에서의 호출이 여전히 컴파일되는지
|
|
- VERSIONING.md에 `## nonce Overflow` 섹션이 추가됐는지, 내용이 "overflow 정의 없음 + 현실적이지 않음" 정책을 명확히 기술하는지
|
|
- `test_communicator.py`에 `test_shutdown_public_method` 추가 및 통과 여부
|
|
|
|
## 검증 결과
|
|
|
|
_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._
|
|
|
|
### REFACTOR-1 중간 검증
|
|
```
|
|
$ grep "TypeScript\|Python" PROTOCOL.md | grep "Planned"
|
|
|
|
$ grep "TypeScript\|Python" PROTOCOL.md | grep "Available"
|
|
| TypeScript | Available | `typescript/` |
|
|
| Python | Available | `python/` |
|
|
```
|
|
|
|
### REFACTOR-2 중간 검증
|
|
```
|
|
$ cd python && python3 -m pytest test/test_communicator.py -v
|
|
============================= test session starts ==============================
|
|
platform linux -- Python 3.12.3, pytest-9.0.3, pluggy-1.6.0 -- /usr/bin/python3
|
|
rootdir: /config/workspace/toki_socket/python
|
|
configfile: pyproject.toml
|
|
plugins: asyncio-1.3.0
|
|
collecting ... collected 5 items
|
|
|
|
test/test_communicator.py::test_add_listener_exclusivity PASSED [ 20%]
|
|
test/test_communicator.py::test_send_and_receive PASSED [ 40%]
|
|
test/test_communicator.py::test_send_request_response PASSED [ 60%]
|
|
test/test_communicator.py::test_shutdown PASSED [ 80%]
|
|
test/test_communicator.py::test_shutdown_public_method PASSED [100%]
|
|
|
|
============================== 5 passed in 0.30s ===============================
|
|
```
|
|
|
|
### REFACTOR-3 중간 검증
|
|
```
|
|
$ cd kotlin && env JAVA_HOME=/config/opt/jdk/jdk-17.0.10+7 GRADLE_USER_HOME=/tmp/gradle ./gradlew test
|
|
> Task :test
|
|
|
|
BUILD SUCCESSFUL in 12s
|
|
10 actionable tasks: 1 executed, 9 up-to-date
|
|
```
|
|
|
|
### REFACTOR-4 중간 검증
|
|
```
|
|
$ grep -A 10 "nonce Overflow" VERSIONING.md
|
|
## nonce Overflow
|
|
|
|
`nonce` and `responseNonce` fields are protobuf `int32` values (signed 32-bit).
|
|
Overflow occurs after about 2.1 billion sends on a single connection.
|
|
|
|
Current policy:
|
|
|
|
- The protocol does not define overflow behavior. Reaching the int32 limit on a single connection is not realistic.
|
|
- Implementations do not add recovery logic for overflow.
|
|
- If overflow handling is needed in the future, the protocol version will be bumped and wrap-around behavior will be specified.
|
|
```
|
|
|
|
### 최종 검증
|
|
```
|
|
$ cd python && python3 -m pytest -v
|
|
============================= test session starts ==============================
|
|
platform linux -- Python 3.12.3, pytest-9.0.3, pluggy-1.6.0 -- /usr/bin/python3
|
|
rootdir: /config/workspace/toki_socket/python
|
|
configfile: pyproject.toml
|
|
plugins: asyncio-1.3.0
|
|
collecting ... collected 7 items
|
|
|
|
test/test_communicator.py::test_add_listener_exclusivity PASSED [ 14%]
|
|
test/test_communicator.py::test_send_and_receive PASSED [ 28%]
|
|
test/test_communicator.py::test_send_request_response PASSED [ 42%]
|
|
test/test_communicator.py::test_shutdown PASSED [ 57%]
|
|
test/test_communicator.py::test_shutdown_public_method PASSED [ 71%]
|
|
test/test_tcp.py::test_tcp_send_receive PASSED [ 85%]
|
|
test/test_tcp.py::test_tcp_request_response PASSED [100%]
|
|
|
|
============================== 7 passed in 0.26s ===============================
|
|
$ cd kotlin && env JAVA_HOME=/config/opt/jdk/jdk-17.0.10+7 GRADLE_USER_HOME=/tmp/gradle ./gradlew test
|
|
> Task :test
|
|
|
|
BUILD SUCCESSFUL in 12s
|
|
10 actionable tasks: 1 executed, 9 up-to-date
|
|
$ grep "TypeScript\|Python" PROTOCOL.md | grep "Available"
|
|
| TypeScript | Available | `typescript/` |
|
|
| Python | Available | `python/` |
|
|
$ grep "nonce Overflow" VERSIONING.md
|
|
## nonce Overflow
|
|
```
|