fix: update communicator implementations and move task docs to archive
This commit is contained in:
parent
3228e759c0
commit
65e4900bd8
7 changed files with 226 additions and 39 deletions
|
|
@ -35,44 +35,45 @@ task=m-protocol-evolution-compatibility/05+04_nonce_dart_go, plan=0, tag=API
|
|||
|
||||
| 항목 | 완료 여부 |
|
||||
|------|---------|
|
||||
| [API-1] Dart pending nonce skip | [ ] |
|
||||
| [API-2] Go pending nonce skip | [ ] |
|
||||
| [API-1] Dart pending nonce skip | [x] |
|
||||
| [API-2] Go pending nonce skip | [x] |
|
||||
|
||||
## 구현 체크리스트
|
||||
|
||||
- [ ] 04번 predecessor `complete.log`가 존재하는지 확인하고, 없으면 구현을 시작하지 않고 리뷰 stub에 차단 근거를 남긴다.
|
||||
- [ ] Dart `Communicator.nextNonce()`가 active `_pendingRequests` 키와 충돌하는 후보 nonce를 건너뛰도록 수정한다.
|
||||
- [ ] Dart `communicator_test.dart`에 wrap 후 `1`이 pending이면 다음 send/request nonce가 `2`가 되는 regression test를 추가한다.
|
||||
- [ ] Go `Communicator`의 nonce 선택이 active `pendingRequests` 키와 충돌하는 후보 nonce를 건너뛰도록 수정한다.
|
||||
- [ ] Go `communicator_nonce_test.go`에 wrap 후 pending nonce skip regression test를 추가한다.
|
||||
- [ ] `cd dart && dart test test/communicator_test.dart`, `cd go && go test ./...`, `git diff --check`를 실행한다.
|
||||
- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다.
|
||||
- [x] 04번 predecessor `complete.log`가 존재하는지 확인하고, 없으면 구현을 시작하지 않고 리뷰 stub에 차단 근거를 남긴다.
|
||||
- [x] Dart `Communicator.nextNonce()`가 active `_pendingRequests` 키와 충돌하는 후보 nonce를 건너뛰도록 수정한다.
|
||||
- [x] Dart `communicator_test.dart`에 wrap 후 `1`이 pending이면 다음 send/request nonce가 `2`가 되는 regression test를 추가한다.
|
||||
- [x] Go `Communicator`의 nonce 선택이 active `pendingRequests` 키와 충돌하는 후보 nonce를 건너뛰도록 수정한다.
|
||||
- [x] Go `communicator_nonce_test.go`에 wrap 후 pending nonce skip regression test를 추가한다.
|
||||
- [x] `cd dart && dart test test/communicator_test.dart`, `cd go && go test ./...`, `git diff --check`를 실행한다.
|
||||
- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다.
|
||||
|
||||
## 코드리뷰 전용 체크리스트
|
||||
|
||||
> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다.
|
||||
> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다.
|
||||
|
||||
- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다.
|
||||
- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다.
|
||||
- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_{review_lane}_GNN_N.log`로 아카이브한다.
|
||||
- [ ] active `PLAN-*-G??.md`를 `plan_{build_lane}_GNN_M.log`로 아카이브한다.
|
||||
- [ ] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다.
|
||||
- [ ] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다.
|
||||
- [ ] PASS이면 active task 디렉터리 `agent-task/{task_name}/`를 `agent-task/archive/YYYY/MM/{task_name}/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다.
|
||||
- [ ] PASS이고 task group이 `m-<milestone-slug>`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다.
|
||||
- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/{task_group}/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다.
|
||||
- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다.
|
||||
- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다.
|
||||
- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_{review_lane}_GNN_N.log`로 아카이브한다.
|
||||
- [x] active `PLAN-*-G??.md`를 `plan_{build_lane}_GNN_M.log`로 아카이브한다.
|
||||
- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다.
|
||||
- [x] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다.
|
||||
- [x] PASS이면 active task 디렉터리 `agent-task/{task_name}/`를 `agent-task/archive/YYYY/MM/{task_name}/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다.
|
||||
- [x] PASS이고 task group이 `m-<milestone-slug>`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다.
|
||||
- [x] PASS split 작업이면 이동 후 빈 active parent `agent-task/{task_group}/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다.
|
||||
- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-{build_lane}-GNN.md`와 `CODE_REVIEW-{review_lane}-GNN.md`를 작성하고 `complete.log`를 작성하지 않는다.
|
||||
- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다.
|
||||
- [ ] USER_REVIEW가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다.
|
||||
|
||||
## 계획 대비 변경 사항
|
||||
|
||||
_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._
|
||||
없음. 계획 명세와 동일하게 구현했다.
|
||||
|
||||
## 주요 설계 결정
|
||||
|
||||
_구현 에이전트가 주요 설계 결정 사항을 기록한다._
|
||||
- **Dart**: `nextNonce()`를 bounded loop(`maxNonce`회)로 교체해 `_pendingRequests.containsKey(candidate)`인 경우 `_nonce`를 계속 진행시킨다. 루프 소진 시 `StateError`를 throw한다. 단일 메서드 수정으로 `send`, `sendRequest`, 자동 응답 모두 커버된다.
|
||||
- **Go**: `nextNonce()` 시그니처를 `(int32, error)`로 변경하고 CAS 성공 후 `c.mu.RLock()`으로 `pendingRequests`를 읽어 충돌 여부를 확인한다. 충돌이면 외부 loop가 다음 후보로 넘어간다. `Send`, `SendRequest`, `AddRequestListenerTyped` 응답 경로 3곳에서 반환된 error를 처리한다.
|
||||
|
||||
## 사용자 리뷰 요청
|
||||
|
||||
|
|
@ -104,21 +105,33 @@ _구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후
|
|||
### API-1 중간 검증
|
||||
```bash
|
||||
$ cd dart && dart test test/communicator_test.dart
|
||||
(output)
|
||||
00:00 +0: loading test/communicator_test.dart
|
||||
00:00 +0: Communicator protocol guards response typeName mismatch completes sendRequest with error
|
||||
00:00 +1: Communicator protocol guards close 후 sendRequest는 StateError로 완료된다
|
||||
00:00 +2: Communicator protocol guards sendRequest가 timeout 내 응답 없으면 TimeoutException을 던진다
|
||||
00:00 +3: Communicator protocol guards nonce wraps after int32 max without emitting zero
|
||||
00:00 +4: Communicator protocol guards nextNonce skips pending nonce after wrap
|
||||
00:00 +5: Communicator protocol guards sendRequest matches response at nonce wrap boundary
|
||||
...
|
||||
00:00 +32: All tests passed!
|
||||
```
|
||||
|
||||
### API-2 중간 검증
|
||||
```bash
|
||||
$ cd go && go test ./...
|
||||
(output)
|
||||
ok git.toki-labs.com/toki/proto-socket/go 0.047s
|
||||
ok git.toki-labs.com/toki/proto-socket/go/test 8.324s
|
||||
```
|
||||
|
||||
### 최종 검증
|
||||
```bash
|
||||
$ cd dart && dart test test/communicator_test.dart
|
||||
00:00 +32: All tests passed!
|
||||
$ cd go && go test ./...
|
||||
ok git.toki-labs.com/toki/proto-socket/go 0.047s
|
||||
ok git.toki-labs.com/toki/proto-socket/go/test 8.324s
|
||||
$ git diff --check
|
||||
(output)
|
||||
(no output — whitespace clean)
|
||||
```
|
||||
|
||||
---
|
||||
|
|
@ -139,3 +152,27 @@ $ git diff --check
|
|||
| 사용자 리뷰 요청 | Implementing agent | Keep `상태: 없음` unless user input is required to proceed |
|
||||
| 리뷰어를 위한 체크포인트 | Fixed at stub creation | Review focus areas |
|
||||
| 검증 결과 | Implementing agent | Fill command output only |
|
||||
|
||||
## 코드리뷰 결과
|
||||
|
||||
- 종합 판정: PASS
|
||||
- 차원별 평가:
|
||||
- correctness: Pass
|
||||
- completeness: Pass
|
||||
- test coverage: Pass
|
||||
- API contract: Pass
|
||||
- code quality: Pass
|
||||
- plan deviation: Pass
|
||||
- verification trust: Pass
|
||||
- 발견된 문제: 없음
|
||||
- 다음 단계: PASS이므로 `complete.log` 작성 후 task directory를 archive로 이동한다. Milestone-linked task 완료 이벤트 메타데이터를 보고하고 roadmap 수정은 runtime에 맡긴다.
|
||||
|
||||
### 리뷰 근거
|
||||
|
||||
- Dart `Communicator.nextNonce()`는 active `_pendingRequests` 키를 건너뛰고, 공통 outbound nonce 경로(`send`, `sendRequest`, 자동 response, heartbeat override)를 기존 시그니처로 유지한다.
|
||||
- Go `Communicator.nextNonce()`는 CAS로 후보를 확정한 뒤 `pendingRequests`를 `RLock`으로 확인하며, `Send`, `SendRequest`, typed request-response path가 새 error 반환을 처리한다.
|
||||
- 추가된 Dart/Go regression tests는 wrap 직후 pending nonce `1`을 유지한 상태에서 다음 outbound packet이 nonce `2`를 쓰는 경계를 재현한다.
|
||||
- 재실행 검증:
|
||||
- `cd dart && dart test test/communicator_test.dart` - PASS; `00:00 +32: All tests passed!`
|
||||
- `cd go && go test ./...` - PASS; root/go and go/test packages passed, remaining packages reported `[no test files]`
|
||||
- `git diff --check` - PASS; output 없음
|
||||
|
|
@ -0,0 +1,37 @@
|
|||
# Complete - m-protocol-evolution-compatibility/05+04_nonce_dart_go
|
||||
|
||||
## 완료 일시
|
||||
|
||||
2026-06-16
|
||||
|
||||
## 요약
|
||||
|
||||
Dart/Go nonce pending collision skip 구현과 regression test 추가를 1회 리뷰 루프로 완료했으며 최종 판정은 PASS다.
|
||||
|
||||
## 루프 이력
|
||||
|
||||
| Plan | Review | Verdict | 메모 |
|
||||
|------|--------|---------|------|
|
||||
| `plan_cloud_G06_0.log` | `code_review_cloud_G06_0.log` | PASS | Dart/Go outbound nonce selection이 active pending request nonce를 건너뛰고 focused 검증이 통과함 |
|
||||
|
||||
## 구현/정리 내용
|
||||
|
||||
- Dart `Communicator.nextNonce()`가 active `_pendingRequests` 키와 충돌하는 candidate nonce를 건너뛰도록 bounded loop를 적용했다.
|
||||
- Dart `communicator_test.dart`에 wrap 후 pending nonce `1`을 유지한 상태에서 다음 outbound nonce가 `2`인지 확인하는 regression test를 추가했다.
|
||||
- Go `Communicator.nextNonce()`를 `(int32, error)`로 확장하고 `pendingRequests` 충돌 candidate를 건너뛰도록 적용했다.
|
||||
- Go `Send`, `SendRequest`, typed request-response path가 nonce exhaustion error를 처리하도록 갱신했다.
|
||||
- Go `communicator_nonce_test.go`에 wrap 후 active pending nonce skip regression test를 추가했다.
|
||||
|
||||
## 최종 검증
|
||||
|
||||
- `cd dart && dart test test/communicator_test.dart` - PASS; `00:00 +32: All tests passed!`
|
||||
- `cd go && go test ./...` - PASS; root/go and go/test packages passed, remaining packages reported `[no test files]`
|
||||
- `git diff --check` - PASS; output 없음
|
||||
|
||||
## 잔여 Nit
|
||||
|
||||
- 없음
|
||||
|
||||
## 후속 작업
|
||||
|
||||
- 없음
|
||||
|
|
@ -67,11 +67,16 @@ abstract class Communicator {
|
|||
|
||||
@protected
|
||||
int nextNonce() {
|
||||
if (_nonce >= maxNonce) {
|
||||
_nonce = 0;
|
||||
for (var i = 0; i < maxNonce; i++) {
|
||||
if (_nonce >= maxNonce) {
|
||||
_nonce = 0;
|
||||
}
|
||||
_nonce += 1;
|
||||
if (!_pendingRequests.containsKey(_nonce)) {
|
||||
return _nonce;
|
||||
}
|
||||
}
|
||||
_nonce += 1;
|
||||
return _nonce;
|
||||
throw StateError('no available nonce: all positive int32 nonces are pending');
|
||||
}
|
||||
|
||||
/// Whether the connection is alive. Set by each transport implementation.
|
||||
|
|
|
|||
|
|
@ -156,6 +156,28 @@ void main() {
|
|||
);
|
||||
});
|
||||
|
||||
test('nextNonce skips pending nonce after wrap', () async {
|
||||
final communicator = _FakeCommunicator();
|
||||
// Wrap to 1 on the next call
|
||||
communicator.setNonceForTest(Communicator.maxNonce);
|
||||
|
||||
final pendingFuture = communicator.sendRequest<TestData, TestData>(
|
||||
TestData()..index = 1,
|
||||
);
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
expect(communicator.transport.sentPackets.single.nonce, 1);
|
||||
|
||||
// Force another wrap so the next nextNonce() candidate is 1 again
|
||||
communicator.setNonceForTest(Communicator.maxNonce);
|
||||
|
||||
// send() must skip nonce 1 (still pending) and use 2
|
||||
await communicator.send(TestData()..index = 2);
|
||||
expect(communicator.transport.sentPackets.last.nonce, 2);
|
||||
|
||||
communicator.closeForTest();
|
||||
await expectLater(pendingFuture, throwsA(isA<StateError>()));
|
||||
});
|
||||
|
||||
test('sendRequest matches response at nonce wrap boundary', () async {
|
||||
final communicator = _FakeCommunicator();
|
||||
communicator.setNonceForTest(Communicator.maxNonce - 1);
|
||||
|
|
|
|||
|
|
@ -184,19 +184,28 @@ func (c *Communicator) onFrameError(err error) {
|
|||
}
|
||||
}
|
||||
|
||||
func (c *Communicator) nextNonce() int32 {
|
||||
for {
|
||||
current := c.nonce.Load()
|
||||
func (c *Communicator) nextNonce() (int32, error) {
|
||||
for i := 0; i < int(MaxNonce); i++ {
|
||||
var next int32
|
||||
if current >= MaxNonce {
|
||||
next = 1
|
||||
} else {
|
||||
next = current + 1
|
||||
for {
|
||||
current := c.nonce.Load()
|
||||
if current >= MaxNonce {
|
||||
next = 1
|
||||
} else {
|
||||
next = current + 1
|
||||
}
|
||||
if c.nonce.CompareAndSwap(current, next) {
|
||||
break
|
||||
}
|
||||
}
|
||||
if c.nonce.CompareAndSwap(current, next) {
|
||||
return next
|
||||
c.mu.RLock()
|
||||
_, taken := c.pendingRequests[next]
|
||||
c.mu.RUnlock()
|
||||
if !taken {
|
||||
return next, nil
|
||||
}
|
||||
}
|
||||
return 0, errors.New("no available nonce: all positive int32 nonces are pending")
|
||||
}
|
||||
|
||||
func (c *Communicator) shutdown() {
|
||||
|
|
@ -300,9 +309,13 @@ func (c *Communicator) Send(m proto.Message) error {
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
n, err := c.nextNonce()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return c.QueuePacket(&packets.PacketBase{
|
||||
TypeName: TypeNameOf(m),
|
||||
Nonce: c.nextNonce(),
|
||||
Nonce: n,
|
||||
Data: data,
|
||||
})
|
||||
}
|
||||
|
|
@ -315,7 +328,10 @@ func (c *Communicator) SendRequest(req proto.Message, resType proto.Message, tim
|
|||
timeout = 30 * time.Second
|
||||
}
|
||||
|
||||
requestNonce := c.nextNonce()
|
||||
requestNonce, err := c.nextNonce()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
pending := &pendingRequest{
|
||||
expectedTypeName: TypeNameOf(resType),
|
||||
ch: make(chan proto.Message, 1),
|
||||
|
|
@ -609,9 +625,13 @@ func AddRequestListenerTyped[Req proto.Message, Res proto.Message](c *Communicat
|
|||
if err != nil {
|
||||
return
|
||||
}
|
||||
n, err := c.nextNonce()
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
_ = c.QueuePacket(&packets.PacketBase{
|
||||
TypeName: TypeNameOf(res),
|
||||
Nonce: c.nextNonce(),
|
||||
Nonce: n,
|
||||
ResponseNonce: requestNonce,
|
||||
Data: data,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -71,6 +71,72 @@ func TestNonceWrapsAfterInt32MaxWithoutEmittingZero(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestNextNonceSkipsActivePendingAfterWrap(t *testing.T) {
|
||||
transport := &nonceTestTransport{}
|
||||
communicator := NewCommunicator(transport, nonceTestParserMap())
|
||||
defer communicator.Close()
|
||||
|
||||
// Set nonce to MaxNonce so the next call wraps to 1
|
||||
communicator.nonce.Store(MaxNonce)
|
||||
|
||||
// Start a request whose nonce will be 1 (after wrap); leave it unanswered
|
||||
reqErrCh := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := SendRequestTyped[*packets.TestData, *packets.TestData](
|
||||
communicator,
|
||||
&packets.TestData{Index: 1, Message: "pending"},
|
||||
time.Second,
|
||||
)
|
||||
reqErrCh <- err
|
||||
}()
|
||||
|
||||
// Wait for the request packet with nonce 1 to be sent
|
||||
var req1 *packets.PacketBase
|
||||
for deadline := time.Now().Add(time.Second); time.Now().Before(deadline); {
|
||||
sent := transport.sent()
|
||||
if len(sent) >= 1 {
|
||||
req1 = sent[0]
|
||||
break
|
||||
}
|
||||
time.Sleep(time.Millisecond)
|
||||
}
|
||||
if req1 == nil {
|
||||
t.Fatal("first request packet not sent")
|
||||
}
|
||||
if req1.GetNonce() != 1 {
|
||||
t.Fatalf("first nonce = %d, want 1", req1.GetNonce())
|
||||
}
|
||||
|
||||
// Reset nonce to MaxNonce to simulate another wrap cycle
|
||||
communicator.nonce.Store(MaxNonce)
|
||||
|
||||
// Send must skip nonce 1 (still pending) and use nonce 2
|
||||
if err := communicator.Send(&packets.TestData{Index: 2}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
sent := transport.sent()
|
||||
if len(sent) < 2 {
|
||||
t.Fatalf("expected at least 2 packets, got %d", len(sent))
|
||||
}
|
||||
if got := sent[len(sent)-1].GetNonce(); got != 2 {
|
||||
t.Fatalf("second nonce = %d, want 2", got)
|
||||
}
|
||||
|
||||
// Resolve the pending request so the goroutine exits cleanly
|
||||
response := &packets.TestData{Index: 99, Message: "done"}
|
||||
data, _ := proto.Marshal(response)
|
||||
communicator.OnReceivedData(TypeNameOf(response), data, 0, req1.GetNonce())
|
||||
|
||||
select {
|
||||
case err := <-reqErrCh:
|
||||
if err != nil {
|
||||
t.Fatalf("pending request error: %v", err)
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("pending request did not complete")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendRequestNonceWrapsAtInt32Max(t *testing.T) {
|
||||
transport := &nonceTestTransport{}
|
||||
communicator := NewCommunicator(transport, nonceTestParserMap())
|
||||
|
|
|
|||
Loading…
Reference in a new issue