From 65e4900bd8ef22215fdb1ae5f3509235a569daf9 Mon Sep 17 00:00:00 2001 From: toki Date: Wed, 17 Jun 2026 07:27:31 +0900 Subject: [PATCH] fix: update communicator implementations and move task docs to archive --- .../code_review_cloud_G06_0.log} | 83 ++++++++++++++----- .../05+04_nonce_dart_go/complete.log | 37 +++++++++ .../05+04_nonce_dart_go/plan_cloud_G06_0.log} | 0 dart/lib/src/communicator.dart | 13 ++- dart/test/communicator_test.dart | 22 +++++ go/communicator.go | 44 +++++++--- go/communicator_nonce_test.go | 66 +++++++++++++++ 7 files changed, 226 insertions(+), 39 deletions(-) rename agent-task/{m-protocol-evolution-compatibility/05+04_nonce_dart_go/CODE_REVIEW-cloud-G06.md => archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/code_review_cloud_G06_0.log} (66%) create mode 100644 agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/complete.log rename agent-task/{m-protocol-evolution-compatibility/05+04_nonce_dart_go/PLAN-cloud-G06.md => archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/plan_cloud_G06_0.log} (100%) diff --git a/agent-task/m-protocol-evolution-compatibility/05+04_nonce_dart_go/CODE_REVIEW-cloud-G06.md b/agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/code_review_cloud_G06_0.log similarity index 66% rename from agent-task/m-protocol-evolution-compatibility/05+04_nonce_dart_go/CODE_REVIEW-cloud-G06.md rename to agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/code_review_cloud_G06_0.log index afa4ac3..8604e72 100644 --- a/agent-task/m-protocol-evolution-compatibility/05+04_nonce_dart_go/CODE_REVIEW-cloud-G06.md +++ b/agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/code_review_cloud_G06_0.log @@ -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-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, 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-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, 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 없음 diff --git a/agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/complete.log b/agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/complete.log new file mode 100644 index 0000000..749a0f4 --- /dev/null +++ b/agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/complete.log @@ -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 + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/m-protocol-evolution-compatibility/05+04_nonce_dart_go/PLAN-cloud-G06.md b/agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/plan_cloud_G06_0.log similarity index 100% rename from agent-task/m-protocol-evolution-compatibility/05+04_nonce_dart_go/PLAN-cloud-G06.md rename to agent-task/archive/2026/06/m-protocol-evolution-compatibility/05+04_nonce_dart_go/plan_cloud_G06_0.log diff --git a/dart/lib/src/communicator.dart b/dart/lib/src/communicator.dart index c7653f6..72c50da 100644 --- a/dart/lib/src/communicator.dart +++ b/dart/lib/src/communicator.dart @@ -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. diff --git a/dart/test/communicator_test.dart b/dart/test/communicator_test.dart index c9cf6e5..9d8912e 100644 --- a/dart/test/communicator_test.dart +++ b/dart/test/communicator_test.dart @@ -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()..index = 1, + ); + await Future.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())); + }); + test('sendRequest matches response at nonce wrap boundary', () async { final communicator = _FakeCommunicator(); communicator.setNonceForTest(Communicator.maxNonce - 1); diff --git a/go/communicator.go b/go/communicator.go index dc36976..6bd3224 100644 --- a/go/communicator.go +++ b/go/communicator.go @@ -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, }) diff --git a/go/communicator_nonce_test.go b/go/communicator_nonce_test.go index 66a70d3..3d59741 100644 --- a/go/communicator_nonce_test.go +++ b/go/communicator_nonce_test.go @@ -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())