From 88c8ff3b07889f8aa14a013bc8df8aa5d9e9bb44 Mon Sep 17 00:00:00 2001 From: toki Date: Sat, 20 Jun 2026 18:23:30 +0900 Subject: [PATCH] feat: runner proto socket transport hardening and related updates - Add runner-proto-socket-transport-hardening milestone and SDD docs - Add runnersocket package for Go service - Update agent config, runner, and job client (Dart) - Update Bootstrap scripts (PowerShell, shell) - Update Go service HTTP server handlers and routes - Add CICD state store updates - Update agent-ops domain rules and phase roadmap --- agent-ops/rules/project/domain/agent/rules.md | 19 +- .../rules/project/domain/framework/rules.md | 4 +- .../control-plane-product-surface/PHASE.md | 3 + ...runner-proto-socket-transport-hardening.md | 73 +++ .../SDD.md | 116 +++++ .../code_review_cloud_G07_0.log | 172 +++++++ .../code_review_local_G06_1.log | 182 +++++++ .../01_socket_lifecycle/complete.log | 51 ++ .../01_socket_lifecycle/plan_cloud_G07_0.log | 187 +++++++ .../01_socket_lifecycle/plan_local_G06_1.log | 116 +++++ .../CODE_REVIEW-cloud-G07.md | 137 ++++++ .../02+01_dispatch_state/PLAN-cloud-G07.md | 175 +++++++ .../CODE_REVIEW-cloud-G07.md | 147 ++++++ .../03+01,02_runner_actions/PLAN-cloud-G07.md | 203 ++++++++ .../CODE_REVIEW-cloud-G07.md | 148 ++++++ .../04+01_compat_boundary/PLAN-cloud-G07.md | 192 ++++++++ .../CODE_REVIEW-cloud-G07.md | 152 ++++++ .../PLAN-cloud-G07.md | 168 +++++++ .../script/powershell/oto_agent_bootstrap.ps1 | 8 + .../script/shell/oto_agent_bootstrap.sh | 13 +- apps/runner/lib/oto/agent/agent_config.dart | 15 +- apps/runner/lib/oto/agent/agent_runner.dart | 53 +- .../lib/oto/agent/oto_server_job_client.dart | 150 +++++- .../lib/oto/agent/registration_client.dart | 193 +++++++- .../lib/oto/agent/remote_run_executor.dart | 55 ++- .../oto_server_connection_smoke_test.dart | 465 ++++++++++++++---- services/core/cmd/oto-core/main.go | 6 + services/core/go.mod | 9 +- services/core/go.sum | 2 + services/core/internal/cicdstate/store.go | 6 +- services/core/internal/httpserver/dto.go | 6 +- .../core/internal/httpserver/job_handlers.go | 5 +- .../httpserver/oto_agent_bootstrap.ps1 | 8 + .../httpserver/oto_agent_bootstrap.sh | 13 +- services/core/internal/httpserver/routes.go | 17 +- .../httpserver/runner_cicd_handlers.go | 44 +- .../internal/httpserver/runner_handlers.go | 34 +- services/core/internal/httpserver/server.go | 89 +++- .../core/internal/httpserver/server_test.go | 12 +- services/core/internal/runnersocket/server.go | 448 +++++++++++++++++ .../core/internal/runnersocket/server_test.go | 207 ++++++++ 41 files changed, 3912 insertions(+), 191 deletions(-) create mode 100644 agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md create mode 100644 agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md create mode 100644 agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_cloud_G07_0.log create mode 100644 agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_local_G06_1.log create mode 100644 agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/complete.log create mode 100644 agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_cloud_G07_0.log create mode 100644 agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_local_G06_1.log create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/CODE_REVIEW-cloud-G07.md create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/PLAN-cloud-G07.md create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/CODE_REVIEW-cloud-G07.md create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/PLAN-cloud-G07.md create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/CODE_REVIEW-cloud-G07.md create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/PLAN-cloud-G07.md create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/CODE_REVIEW-cloud-G07.md create mode 100644 agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/PLAN-cloud-G07.md create mode 100644 services/core/internal/runnersocket/server.go create mode 100644 services/core/internal/runnersocket/server_test.go diff --git a/agent-ops/rules/project/domain/agent/rules.md b/agent-ops/rules/project/domain/agent/rules.md index e1baa80..0e31b89 100644 --- a/agent-ops/rules/project/domain/agent/rules.md +++ b/agent-ops/rules/project/domain/agent/rules.md @@ -8,7 +8,7 @@ last_rule_updated_at: 2026-06-13 ## 목적 / 책임 -OTO Server에 CLI 기반 runner agent를 등록하고, agent config 파싱, 등록/heartbeat/disconnect, remote job claim, remote pipeline 실행, 실행 결과/로그/artifact 보고, bootstrap 스크립트를 담당한다. +OTO Server에 CLI 기반 runner agent를 등록하고, agent config 파싱, proto-socket runner session, remote pipeline 실행, 실행 결과/로그/artifact 보고, bootstrap 스크립트를 담당한다. 파이프라인 실행 엔진이나 scheduler 등록 정책이 아니라 `oto agent run --config ` 실행 흐름과 OTO Server runner 세션 초기화가 이 도메인의 책임이다. ## 포함 경로 @@ -39,13 +39,14 @@ OTO Server에 CLI 기반 runner agent를 등록하고, agent config 파싱, 등 - `AgentConfigException` — config 파일/필드/타입 오류 표현 - `AgentRunner` / `DefaultAgentRunner` — OTO Server 등록 workflow, remote job loop, 결과 로그 출력 - `RegistrationException` — OTO Server 등록 거절 또는 연결 실패 표현 -- `RegistrationClient` / `AgentSession` / `OtoServerJobSession` — 등록 transport와 세션 수명주기 추상화 -- `OtoServerRegistrationClient` — HTTP 기반 runner register, heartbeat, disconnect 요청 -- `OtoServerJobClient` — job claim, execution report, log append, artifact report, cancel, status, self-update 요청 +- `RegistrationClient` / `AgentSession` / `OtoServerPushJobSession` / `OtoServerJobSession` — 등록 transport와 세션 수명주기 추상화. 기본 OTO Server runner 경로는 proto-socket push session이다. +- `OtoServerSocketRegistrationClient` / `OtoServerSocketJobClient` — OTO Server proto-socket 등록, heartbeat, pushed `RunRequest` 수신, execution/log/artifact 보고 +- `OtoServerRegistrationClient` / `OtoServerJobClient` — HTTP 기반 legacy/compatibility 경로. bootstrap, UI API smoke, 기존 HTTP 계약 확인에 남기되 기본 production runner loop로 확장하지 않는다. +- `RemoteJobReporter` — remote run 실행 결과를 HTTP/socket transport와 분리해 보고하는 interface - `RemoteRunExecutor` / `RunRequestParser` / `RemoteRunInput` — OTO Server `RunRequest`를 workspace-confined YAML 실행 입력으로 변환하고 `Application.build()` 결과를 보고 - `RunnerCapabilityProvider` / `RunnerCapabilitySnapshot` — 등록 시 runner capability와 command catalog 요약 제공 계약 - `RegistrationResult` — OTO Server/iop response의 accepted/rejected 결과 변환 -- `EdgeEndpoint` / `EdgeRegistrationClient` / `_OtoIopClient` — legacy iop Edge compatibility 경로. 기본 runner 경로에서는 `OtoServerRegistrationClient`를 사용 +- `EdgeEndpoint` / `EdgeRegistrationClient` / `_OtoIopClient` — legacy iop Edge compatibility 경로. 기본 runner 경로에서는 `OtoServerSocketRegistrationClient`를 사용 - `apps/runner/lib/oto/agent/oto/*.pb*.dart` — `proto/oto/runner.proto` generated Dart protobuf 코드 - `apps/runner/lib/oto/agent/iop/*.pb*.dart` — legacy `iop/runtime.proto` generated protobuf 코드 - `oto_agent_bootstrap.sh` — agent config 파일 생성, 권한 설정, background 실행 bootstrap @@ -56,15 +57,15 @@ OTO Server에 CLI 기반 runner agent를 등록하고, agent config 파싱, 등 - `--config `와 `--config=` 형식을 모두 유지하고, 누락/알 수 없는 인자는 실행 전에 오류 처리한다. - `AgentConfig`는 YAML root와 `agent`, `server`(또는 legacy `edge`), `runtime` section이 map인지 확인한 뒤 required string을 검증한다. - agent config의 required field는 빈 문자열을 거부하고, optional `agent.alias`는 누락 시 `null`, 빈 문자열이면 빈 문자열로 보존한다. -- OTO Server URL은 scheme 없는 host도 허용하고, HTTP endpoint 생성 시 기본 `http://`로 해석한다. -- OTO Server 등록 세션은 `openSession()`에서 열고 세션 소유자가 `finally`에서 `AgentSession.close()`를 호출해 heartbeat timer와 HTTP client를 정리한다. -- 기본 agent loop는 job claim 실패를 "대기할 작업 없음"으로 보고 polling을 계속하고, claim된 `RunRequest`는 `RemoteRunExecutor.runOnce()`로 실행한다. +- OTO Server URL은 scheme 없는 host도 허용하고, HTTP endpoint 생성 시 기본 `http://`로 해석한다. runner socket URL은 `server.socket_url`을 우선하고, 없으면 server host의 `tcp://:18080`으로 해석한다. +- OTO Server 등록 세션은 `openSession()`에서 열고 세션 소유자가 `finally`에서 `AgentSession.close()`를 호출해 heartbeat timer와 transport를 정리한다. +- 기본 agent loop는 proto-socket으로 pushed `RunRequest`를 받아 `RemoteRunExecutor.runOnce()`로 실행한다. HTTP `jobs/claim` polling loop는 compatibility fallback으로만 유지한다. - remote YAML path는 `workspaceRoot` 아래로 confinement를 강제하고, inline YAML이 있으면 path보다 우선한다. - remote variables는 YAML `property` map 위에 overlay하되, 기존 `property`가 map이 아니면 원본 YAML을 유지해 `Application.build()` 검증이 실패하게 한다. - `property.artifacts` 선언은 `name`/`path` map list만 허용하고, artifact path도 workspace confinement를 통과해야 한다. - RegisterResponse/JSON 변환은 `RegistrationResult`에 모아 accepted/rejected와 runner id/alias 추출을 한 곳에서 처리한다. - generated protobuf 파일은 직접 수정하지 않고 원본 proto와 생성 절차를 확인해 재생성한다. -- bootstrap 스크립트는 `--server-url`을 기본 인자로 사용하고 legacy `--edge-url`도 server URL로 받는다. +- bootstrap 스크립트는 `--server-url`과 optional `--socket-url`을 기본 인자로 사용하고 legacy `--edge-url`도 server URL로 받는다. - bootstrap 스크립트는 `--release-base-url`에 https만 허용하고, 생성한 config 파일은 `chmod 600`을 유지한다. - legacy iop Edge 경로는 `edge_registration_client.dart` 안에 격리하고, 기본 production runner 경로에서 직접 import하지 않는다. - agent 변경 후 `dart analyze`와 agent 관련 테스트(`apps/runner/test/oto_agent_*`, `apps/runner/test/oto_server_connection_smoke_test.dart`, 필요 시 `apps/runner/test/oto_iop_connection_smoke_test.dart`)를 확인한다. diff --git a/agent-ops/rules/project/domain/framework/rules.md b/agent-ops/rules/project/domain/framework/rules.md index 8eac236..9aee591 100644 --- a/agent-ops/rules/project/domain/framework/rules.md +++ b/agent-ops/rules/project/domain/framework/rules.md @@ -41,7 +41,7 @@ last_rule_updated_at: 2026-06-13 - `dart_framework/platform/isolate_manager.dart` — CLI 실행/스케줄러 isolate 처리에서 사용 - `dart_framework/utils/*` — `simpleFuture`, path/system/string/os startup 유틸 사용 - `dart_framework/log/log.dart` — `Application.logWithType()`의 로그 타입에 사용 -- `proto_socket` — legacy iop Edge compatibility 경로에서 사용하는 workspace 상위 path dependency +- `proto_socket` — 기본 OTO Server runner session과 legacy iop Edge compatibility 경로에서 사용하는 workspace 상위 path dependency - `protobuf` / `fixnum` — generated protobuf 코드 런타임 의존성 - `resource_importer` — `apps/runner/lib/resources.resource_importer.dart` 생성 파일과 asset 임베딩 설정 - `proto/oto/runner.proto` — OTO Server runner 등록, heartbeat, bootstrap, job claim, execution/log/artifact report, cancel/status/self-update 계약 @@ -64,7 +64,7 @@ last_rule_updated_at: 2026-06-13 - **core**: `apps/runner/lib/oto/application.dart`는 OTO 오케스트레이터이며 외부 framework의 Application과 별개다 - **cli/scheduler**: isolate, process, OS startup 유틸을 소비하지만 스케줄러 정책은 scheduler 도메인에 둔다 - **command**: 커맨드들은 `ProcessExecutor`를 소비하지만 커맨드별 비즈니스 로직은 command 도메인에 둔다 -- **agent**: `http`, `proto_socket`, `protobuf`, `fixnum` 의존성은 framework 도메인에서 관리하지만 OTO Server/legacy iop protocol 사용 방식과 agent 정책은 agent 도메인에 둔다 +- **agent**: `http`, `proto_socket`, `protobuf`, `fixnum` 의존성은 framework 도메인에서 관리하지만 OTO Server runner socket/HTTP compatibility/legacy iop protocol 사용 방식과 agent 정책은 agent 도메인에 둔다 - **services/core/internal**: Go core service business logic은 아직 dedicated domain rule이 없다. framework 도메인은 `go.mod`, `go.sum`, generated proto, build/test command 경계만 다룬다 ## 금지 사항 diff --git a/agent-roadmap/phase/control-plane-product-surface/PHASE.md b/agent-roadmap/phase/control-plane-product-surface/PHASE.md index c1ea6f4..65b504d 100644 --- a/agent-roadmap/phase/control-plane-product-surface/PHASE.md +++ b/agent-roadmap/phase/control-plane-product-surface/PHASE.md @@ -27,6 +27,9 @@ - [완료] Control Plane 운영 액션 표면 - 경로: `agent-roadmap/archive/phase/control-plane-product-surface/milestones/control-plane-operator-actions.md` - 요약: 기존 OTO Core write endpoint를 UI/adapter action 표면으로 연결하되, 상태 전이와 실패/확인 UX는 SDD로 먼저 잠근다. +- [진행중] Runner proto-socket transport hardening + - 경로: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` + - 요약: OTO Server와 runner의 기본 내부 연결을 proto-socket push/session 기준으로 다듬고, HTTP job claim/polling 잔존 경로를 compatibility 경계로 축소한다. - [스케치] Control Plane 운영 보안·감사 표면 - 경로: `agent-roadmap/phase/control-plane-product-surface/milestones/control-plane-security-audit-surface.md` - 요약: write action 표면 이후 필요한 인증/권한, 감사 로그, 민감 정보 마스킹, UI 노출 경계를 SDD와 구현 가능한 Task로 구체화한다. diff --git a/agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md b/agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md new file mode 100644 index 0000000..2480dc0 --- /dev/null +++ b/agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md @@ -0,0 +1,73 @@ +# Milestone: Runner proto-socket transport hardening + +## 위치 + +- Roadmap: `agent-roadmap/ROADMAP.md` +- Phase: `agent-roadmap/phase/control-plane-product-surface/PHASE.md` + +## 목표 + +OTO Server와 runner 사이의 기본 내부 연결을 proto-socket 장기 세션으로 고정하고, 급하게 붙인 HTTP job claim/polling 잔존 경로를 compatibility 경계로 격리한다. +완료 후 bootstrap으로 연결된 runner는 register, heartbeat, pushed RunRequest, execution/log/artifact report를 socket 기준으로 검증하며, cancel/status/self-update의 방향성과 실패 처리가 문서와 테스트에서 같은 계약을 바라봐야 한다. + +## 상태 + +[진행중] + +## 구현 잠금 + +- 상태: 해제 +- SDD: 필요 +- SDD 문서: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- SDD 사유: server-runner 장기 연결, lifecycle/state machine, cancel/self-update 방향성, HTTP compatibility 축소는 API/proto/config/env와 field smoke에 영향을 주므로 구현 전에 계약을 고정해야 한다. +- 잠금 해제 조건: + - [x] SDD 상태가 `[승인됨]`이고 SDD 잠금이 해제되어 있다. + - [x] SDD `USER_REVIEW.md`가 없거나 승인/해결되었다. + - [x] Acceptance Scenario가 아래 기능 Task와 연결되어 있다. + - [x] Evidence Map이 plan의 `Spec Targets`와 완료 시 `Spec Completion`으로 검증 가능하게 연결되어 있다. +- 결정 필요: + - 없음. 기본 방향은 `proto-socket` push/session을 production runner transport로 두고, HTTP runner job claim/polling은 compatibility 경계로 축소한다. + +## 범위 + +- OTO Server runner socket listener와 Dart runner socket client/session의 register, heartbeat, run dispatch, report, disconnect lifecycle을 정리한다. +- HTTP job claim/report/log/artifact 경로는 public operator API와 runner transport compatibility를 분리해 문서, 명명, 테스트에서 기본 경로로 보이지 않게 한다. +- cancel/status/self-update가 socket push/session과 HTTP operator action 사이에서 어떤 방향으로 흐르는지 고정한다. +- bootstrap/config/env에서 `server.url`과 `server.socket_url`의 책임을 분리하고 기본 포트/공개 URL 계산을 검증한다. + +## 기능 + +### Epic: [socket-transport] Runner socket transport 정리 + +기본 runner transport를 proto-socket으로 고정하고, HTTP polling 잔재를 compatibility 경계로 격리한다. + +- [x] [socket-contract] register, heartbeat, RunRequest push, execution/log/artifact report의 server-runner 방향성과 메시지 계약을 SDD와 domain rule에 고정한다. 검증: SDD Acceptance Scenario와 domain rule이 HTTP polling을 기본 경로로 설명하지 않는다. +- [ ] [session-lifecycle] runner id reconciliation, duplicate connection close, disconnect/timeout, reconnect 후보, heartbeat 실패 처리의 socket session lifecycle을 정리한다. 검증: Go/Dart unit 또는 smoke가 server-assigned runner id와 disconnect 상태 전이를 확인한다. +- [ ] [dispatch-state] queued job dispatch, invalid RunRequest 실패 처리, active execution gate, terminal state 전이를 socket dispatch 기준으로 정리한다. 검증: socket dispatch 테스트가 queued -> running -> terminal 및 invalid input -> failed를 확인한다. +- [ ] [runner-actions] cancel/status/self-update의 operator HTTP action과 runner socket message 방향을 정리하고 구현 또는 명시적 범위 제외로 고정한다. 검증: cancel이 runner 실행과 store state 사이에서 모순된 terminal report를 만들지 않는다. +- [ ] [compat-boundary] HTTP runner job claim/report/log/artifact 경로를 compatibility로 격리하고 기본 agent loop와 기본 smoke에서 제거한다. 검증: `OtoServerSocketRegistrationClient`가 기본이고 HTTP polling 테스트는 compatibility 이름/그룹으로만 남는다. +- [ ] [socket-smoke] local/remote smoke 기준을 proto-socket 연결 중심으로 갱신한다. 검증: `cd services/core && go test ./...`, `cd apps/runner && dart analyze`, agent socket smoke test가 통과한다. + +## 완료 리뷰 + +- 상태: 없음 +- 요청일: 없음 +- 완료 근거: 없음 +- 검토 항목: 없음 +- 리뷰 코멘트: 없음 + +## 범위 제외 + +- iop Edge 직접 연결 경로를 복원하지 않는다. +- production-grade 인증/권한/감사 로그는 `Control Plane 운영 보안·감사 표면` Milestone에서 다룬다. +- durable artifact storage와 장기 job scheduler policy는 이 Milestone에서 완성하지 않는다. +- HTTP public operator API 전체를 제거하지 않는다. 제거/축소 대상은 runner 내부 transport로 쓰이던 HTTP job claim/polling 경로다. + +## 작업 컨텍스트 + +- 관련 경로: `services/core/internal/runnersocket/**`, `services/core/internal/httpserver/**`, `services/core/cmd/oto-core/**`, `apps/runner/lib/oto/agent/**`, `apps/runner/assets/script/**/oto_agent_bootstrap.*`, `proto/oto/runner.proto`, `agent-test/local/agent-smoke.md` +- 표준선(선택): server/operator-facing HTTP API는 유지하고, runner-facing 내부 실행 transport는 proto-socket으로 둔다. +- 표준선(선택): HTTP runner claim/report 코드는 삭제 전 compatibility fallback으로 남기더라도 기본 agent loop, bootstrap, smoke 기준에서는 제외한다. +- 선행 작업: `Runner bootstrap, OTO binary, 노드 연결`, `Control Plane API 데이터 바인딩`, `Control Plane 운영 액션 표면` +- 후속 작업: 운영 보안·감사 표면, durable persistence, 운영 대시보드 고도화 +- 확인 완료: SDD에서 cancel/self-update의 즉시 구현 범위와 compatibility endpoint 제거/유지 기준을 검증 가능한 acceptance로 고정했다. diff --git a/agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md b/agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md new file mode 100644 index 0000000..75b910f --- /dev/null +++ b/agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md @@ -0,0 +1,116 @@ +# SDD: Runner proto-socket transport hardening + +## 위치 + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Phase: `agent-roadmap/phase/control-plane-product-surface/PHASE.md` + +## 상태 + +[승인됨] + +## SDD 잠금 + +- 상태: 해제 +- 사용자 리뷰: 없음 +- 잠금 항목: + - 없음 + +## 문제 / 비목표 + +- 문제: OTO runner transport가 proto-socket push/session과 HTTP claim/polling compatibility 경로를 함께 갖고 있어, 기본 production runner loop, 상태 전이, cancel/status/self-update 방향, smoke evidence의 source of truth가 흐려질 수 있다. +- 비목표: + - HTTP public operator API 전체 제거 + - production-grade 인증/권한/감사 로그 + - durable artifact storage와 장기 scheduler policy + - iop Edge 직접 연결 경로 복원 + +## Source of Truth + +| 영역 | 기준 | 메모 | +|------|------|------| +| Roadmap | `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` | Milestone 기능 Task와 완료 반영 기준 | +| Code | `services/core/internal/runnersocket/server.go` | server-runner proto-socket register, heartbeat, RunRequest dispatch, report, cancel push 기준 | +| Code | `apps/runner/lib/oto/agent/registration_client.dart` | Dart runner socket session, heartbeat, pushed RunRequest 수신 기준 | +| Code | `apps/runner/lib/oto/agent/agent_runner.dart` | 기본 agent loop가 socket push session을 우선 소비하는 기준 | +| Code | `services/core/internal/httpserver/routes.go` | operator-facing HTTP API와 compatibility runner HTTP endpoint 경계 | +| Code | `proto/oto/runner.proto` | wire message 이름과 필드 계약 | +| Domain Rule | `agent-ops/rules/project/domain/agent/rules.md` | 기본 runner 경로는 proto-socket이고 HTTP polling은 compatibility fallback이라는 정책 기준 | +| External Provider | 없음 | 이 Milestone은 외부 provider write를 추가하지 않는다. | +| User Decision | 없음 | 기본 방향과 범위는 Milestone 및 domain rule로 결정되어 있다. | + +## State Machine + +| 상태 | 진입 조건 | 다음 상태 | 근거 | +|------|-----------|-----------|------| +| runner.accepted | `RegisterRunnerRequest`가 enrollment token, runner id, protocol version, capability 검증을 통과한다. | runner.online, runner.disconnected, runner.heartbeat_timeout | `runnerregistry.Register`, `runnersocket.RegisterRunnerRequest` handler | +| runner.online | socket session에서 register 직후 또는 heartbeat가 성공한다. | runner.disconnected, runner.heartbeat_timeout | `runnerregistry.Heartbeat`, socket disconnect listener, timeout loop | +| runner.disconnected | 현재 socket client가 닫히고 server가 해당 client를 현재 runner client로 보고 있다. | runner.accepted | duplicate/reconnect는 re-register를 통해 새 accepted record를 만든다. | +| job.queued | operator HTTP API가 `run_request`를 포함한 job을 생성한다. | job.running, job.failed, job.canceled | `cicdstate.Store`, socket dispatcher | +| job.running | socket dispatcher가 queued job을 claim하고 execution을 생성한 뒤 `RunRequest`를 push한다. | job.succeeded, job.failed, job.canceled | `runnersocket.DispatchQueuedJobs`, `handleExecutionReport`, `CancelJobExecution` | +| execution.running | job dispatch가 runner id를 execution owner로 고정하고 `RunRequest`를 runner에 보낸다. | execution.succeeded, execution.failed, execution.canceled | `cicdstate.Store`, runner report/cancel handlers | +| execution.canceled | operator HTTP cancel action이 store를 canceled로 전환하고 연결된 runner socket에 best-effort cancel message를 보낸다. | terminal | terminal report는 store transition guard와 report handler에서 모순 없이 거부/기록되어야 한다. | + +## Interface Contract + +- 계약 원문: 없음. Wire source는 `proto/oto/runner.proto`이며 SDD에는 원문을 복제하지 않는다. +- 입력: + - `RegisterRunnerRequest`: runner identity, enrollment token, protocol version, capability, command catalog. + - `HeartbeatRequest`: runner liveness와 health status. + - `RunRequest`: server가 socket으로 runner에 push하는 job/execution 실행 입력. `pipeline_yaml` 또는 `pipeline_yaml_path` 중 정확히 하나만 유효하다. + - `ExecutionReportRequest`: runner가 socket으로 보고하는 terminal execution 결과. + - `LogAppendRequest` / `ArtifactReportRequest`: runner가 socket으로 보고하는 execution log/artifact metadata. + - `CancelRunRequest`: operator HTTP cancel action 이후 server가 연결된 runner socket에 best-effort로 전달하는 cancel signal. + - `RunnerStatusRequest` / `SelfUpdateRequest`: proto에는 남아 있지만 이 Milestone에서는 status source와 self-update 실행 범위를 명시적으로 고정한 뒤 구현 또는 범위 제외한다. +- 출력: + - register/heartbeat/report/log/artifact response는 accepted/success 여부와 structured error를 반환한다. + - operator-facing HTTP status/self-update response는 UI/operator가 읽는 상태와 deferral 결과를 반환한다. +- 금지: + - 기본 agent loop를 HTTP `jobs/claim` polling 확장 대상으로 삼지 않는다. + - HTTP runner job claim/report/log/artifact 경로를 기본 smoke나 production runner transport로 설명하지 않는다. + - terminal execution/job 상태를 cancel 이후 runner report로 되돌리지 않는다. + - generated protobuf 파일을 수동 수정하지 않는다. + +## Acceptance Scenarios + +| ID | Milestone Task | Given | When | Then | +|----|----------------|-------|------|------| +| S01 | `socket-contract` | Milestone과 agent domain rule이 proto-socket 기본 경로를 선언한다. | SDD와 domain rule을 확인한다. | SDD Acceptance Scenario와 domain rule 모두 HTTP polling을 기본 runner path로 설명하지 않는다. | +| S02 | `session-lifecycle` | 같은 runner id가 재연결하거나 heartbeat/disconnect가 발생한다. | socket server가 register, heartbeat, duplicate client close, disconnect/timeout을 처리한다. | server-assigned runner id reconciliation과 disconnect 상태 전이가 Go/Dart unit 또는 smoke에서 검증된다. | +| S03 | `dispatch-state` | queued job과 invalid `RunRequest` 후보가 있다. | socket dispatcher가 idle runner에 `RunRequest`를 push한다. | queued -> running -> terminal, invalid input -> failed, active execution gate가 socket dispatch 테스트로 검증된다. | +| S04 | `runner-actions` | operator가 cancel/status/self-update HTTP action을 호출한다. | server가 store, registry, runner socket message 방향을 적용한다. | cancel은 runner 실행과 store state 사이에 모순된 terminal report를 만들지 않고, status/self-update 범위가 구현 또는 명시적 범위 제외로 고정된다. | +| S05 | `compat-boundary` | HTTP runner claim/report/log/artifact compatibility 코드가 남아 있다. | 기본 agent loop, smoke, 테스트 이름/그룹을 정리한다. | `OtoServerSocketRegistrationClient`가 기본이고 HTTP polling 테스트는 compatibility 이름/그룹으로만 남는다. | +| S06 | `socket-smoke` | local/remote smoke 기준이 runner transport 완료 근거로 쓰인다. | agent smoke 문서와 테스트가 socket 연결 중심 evidence를 요구한다. | `cd services/core && go test ./...`, `cd apps/runner && dart analyze`, agent socket smoke test가 통과한다. | + +## Evidence Map + +| Scenario | Required Evidence | `agent-task` 연결 | `Spec Completion` 기대 | +|----------|-------------------|------------------|---------------------------| +| S01 | `agent-roadmap/sdd/.../SDD.md`와 `agent-ops/rules/project/domain/agent/rules.md`가 proto-socket 기본 경로와 HTTP compatibility 경계를 같은 방향으로 설명한다. | roadmap-only update | `socket-contract`: SDD와 domain rule에서 HTTP polling이 기본 경로가 아님을 확인 | +| S02 | Go/Dart unit 또는 smoke가 server-assigned runner id, duplicate connection close, disconnect/timeout, heartbeat failure를 검증한다. | `agent-task/m-runner-proto-socket-transport-hardening/01_socket_lifecycle` | `session-lifecycle`: lifecycle 상태 전이와 재연결 기준 검증 | +| S03 | socket dispatch 테스트가 queued -> running -> terminal, invalid input -> failed, active execution gate를 검증한다. | `agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state` | `dispatch-state`: socket dispatch 상태 전이 검증 | +| S04 | cancel/status/self-update 테스트가 operator HTTP action과 socket message/store state 방향을 검증하거나 self-update 실행 범위 제외를 명시한다. | `agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions` | `runner-actions`: cancel terminal report 모순 방지와 action 범위 고정 | +| S05 | search/test evidence가 기본 agent loop와 기본 smoke에서 HTTP polling claim/report를 제거하고 compatibility 그룹으로만 남겼음을 보여준다. | `agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary` | `compat-boundary`: 기본 runner transport와 compatibility test naming 분리 | +| S06 | agent socket smoke와 local test rule/profile evidence가 socket-centered smoke 기준을 요구하고 통과한다. | `agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke` | `socket-smoke`: Go test, Dart analyze, agent socket smoke 통과 | + +## Cross-repo Dependencies + +- 없음 + +## Drift Check + +- [x] Milestone 기능 Task와 Acceptance Scenario가 일치한다. +- [x] Evidence Map이 plan/code-review/complete.log에서 검증 가능하다. +- [x] agent-contract를 쓰는 경우 SDD에 계약 원문을 복제하지 않았다. +- [x] 사용자 리뷰가 필요한 항목은 `USER_REVIEW.md`에만 남겼다. + +## 사용자 리뷰 이력 + +- 없음 + +## 작업 컨텍스트 + +- 표준선: server/operator-facing HTTP API는 유지하고, runner-facing 내부 실행 transport는 proto-socket push/session으로 둔다. +- 표준선: HTTP runner claim/report/log/artifact 경로는 삭제 전 compatibility fallback으로만 남기며, 기본 agent loop와 기본 smoke evidence에서는 제외한다. +- 표준선: cancel은 operator HTTP action에서 store를 terminal canceled로 바꾸고 연결된 socket runner에 best-effort signal을 보낸다. runner-side cooperative stop이 부족하면 그 구현을 `runner-actions` plan에서 다루고, self-update의 실제 binary replacement는 범위 제외 또는 후속 Milestone으로 고정한다. +- 후속 SDD: 없음 diff --git a/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_cloud_G07_0.log b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_cloud_G07_0.log new file mode 100644 index 0000000..523d065 --- /dev/null +++ b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_cloud_G07_0.log @@ -0,0 +1,172 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service blockers, generic scope changes, repeated failures, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only SDD/Milestone lock decisions in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-20 +task=m-runner-proto-socket-transport-hardening/01_socket_lifecycle, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `session-lifecycle`: runner id reconciliation, duplicate connection close, disconnect/timeout, reconnect 후보, heartbeat 실패 처리의 socket session lifecycle을 정리한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S02`: task=`session-lifecycle`; evidence=`Go/Dart unit 또는 smoke가 server-assigned runner id, duplicate connection close, disconnect/timeout, heartbeat failure를 검증한다.` +- Completion mode: spec-check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G07.md` → `code_review_cloud_G07_N.log`, `PLAN-cloud-G07.md` → `plan_cloud_G07_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/`로 이동한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] 서버 socket lifecycle 테스트와 보강 | [x] | +| [API-2] Dart socket session heartbeat/close 테스트와 보강 | [x] | + +## 구현 체크리스트 + +- [x] [API-1] 서버 socket lifecycle 테스트와 필요한 보강을 구현한다. 검증: `cd services/core && go test -count=1 ./...` +- [x] [API-2] Dart socket session heartbeat/close behavior 테스트와 필요한 보강을 구현한다. 검증: `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` +- [x] 전체 agent 검증을 실행한다. 검증: `cd apps/runner && dart analyze` +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G07_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_cloud_G07_M.log`로 아카이브한다. +- [x] `.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/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/`를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-runner-proto-socket-transport-hardening/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [x] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active plan/review 파일 또는 follow-up plan을 작성하고 `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`를 남기지 않는다. + +## 계획 대비 변경 사항 + +1. **`removeClient` bool 반환값 추가**: 계획은 stale disconnect를 guard하도록 요구했다. 기존 `removeClient`는 void였으나 `bool` 반환으로 변경해 disconnect listener에서 `registry.Disconnect`를 조건부 호출하도록 보강했다. + +2. **`_runRequests.close()` await 제거**: `_OtoServerSocketAgentSession.close()`에서 `await _runRequests.close()`가 구독자 없을 때 무기한 대기하는 Dart single-subscription StreamController 특성 때문에 hang이 발생했다. `unawaited(_runRequests.close())`로 변경해 구독자 유무와 무관하게 close()가 완료되도록 했다. + +3. **두 번째 Dart 소켓 테스트 추가**: PLAN은 한 개의 테스트를 제안했으나, server disconnect 후 idempotent close 시나리오를 추가 테스트로 분리해 각각의 검증 범위를 명확히 했다. + +## 주요 설계 결정 + +1. **`removeClient(runnerID, client) → bool` 패턴**: stale disconnect 보호를 pointer 비교로 구현했다. 같은 runnerID라도 다른 `*protoSocket.TcpClient` 인스턴스이면 false 반환 → `registry.Disconnect` 생략. 이렇게 해야 reconnect 중 old client의 disconnect event가 new session을 덮지 않는다. + +2. **`net.Pipe()` 기반 Go unit test**: real TCP server 없이 `net.Pipe()`로 TcpClient를 생성해 `setClient/removeClient/connectedRunnerIDs` 등 server의 내부 메서드를 같은 package(runnersocket)에서 직접 테스트했다. heartbeat를 비활성(intervalSec=0)으로 설정해 배경 타이머 간섭을 차단했다. + +3. **`_startCoreWithSocket` helper**: Dart 소켓 테스트는 real Go server가 필요하므로 OTO_RUNNER_SOCKET_ADDR 환경변수로 proto-socket 포트를 별도 할당하는 helper를 추가했다. HTTP smoke 테스트의 기존 `_waitForPort` 패턴을 재사용해 server ready 확인을 일관되게 처리했다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- stale socket disconnect가 current runner session을 disconnected로 덮지 않는지 확인한다. +- heartbeat failure/close 테스트가 timer leak 없이 끝나는지 확인한다. +- Roadmap Targets와 Spec Targets가 실제 구현 evidence와 맞는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### API-1 중간 검증 +```bash +$ cd services/core && go test -count=1 ./... +? github.com/toki/oto/services/core/cmd/oto-core [no test files] +ok github.com/toki/oto/services/core/internal/cicdstate 0.002s +ok github.com/toki/oto/services/core/internal/httpserver 0.100s +ok github.com/toki/oto/services/core/internal/runnerregistry 0.005s +ok github.com/toki/oto/services/core/internal/runnersocket 0.029s +? github.com/toki/oto/services/core/oto [no test files] +``` + +### API-2 중간 검증 +```bash +$ cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +00:07 +50: All tests passed! +``` + +### 최종 검증 +```bash +$ cd services/core && go test -count=1 ./... +? github.com/toki/oto/services/core/cmd/oto-core [no test files] +ok github.com/toki/oto/services/core/internal/cicdstate 0.002s +ok github.com/toki/oto/services/core/internal/httpserver 0.100s +ok github.com/toki/oto/services/core/internal/runnerregistry 0.005s +ok github.com/toki/oto/services/core/internal/runnersocket 0.029s +? github.com/toki/oto/services/core/oto [no test files] +$ cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +00:07 +50: All tests passed! +$ cd apps/runner && dart analyze +Analyzing runner... +No issues found! +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 코드리뷰 결과 + +- 종합 판정: FAIL +- 차원별 평가: + - correctness: Fail + - completeness: Fail + - test coverage: Fail + - API contract: Fail + - code quality: Pass + - plan deviation: Pass + - verification trust: Fail + - spec conformance: Fail +- 발견된 문제: + - Required: `services/core/internal/runnersocket/server.go:350`의 `setClient`가 `s.mu`를 잡은 상태에서 old `TcpClient.Close()`를 호출합니다. production client에는 `attachClientHandlers`가 등록한 disconnect listener가 있고, proto-socket `Close()`는 `notifyDisconnected()`에서 listener를 동기 호출합니다. 따라서 duplicate registration에서 old client close -> listener -> `removeClient()` -> 같은 `s.mu` 재획득 경로로 교착될 수 있습니다. 수정은 새 client를 map에 저장하고 lock을 푼 뒤 old client를 close하도록 바꾸며, `attachClientHandlers`가 붙은 client 2개로 duplicate replacement가 완료되는 단위 테스트를 추가해야 합니다. +- 다음 단계: + - FAIL: user-review gate는 트리거하지 않고, 위 Required를 해결하는 후속 `PLAN-local-G06.md` / `CODE_REVIEW-local-G06.md`를 작성한다. diff --git a/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_local_G06_1.log b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_local_G06_1.log new file mode 100644 index 0000000..1dbdfcc --- /dev/null +++ b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_local_G06_1.log @@ -0,0 +1,182 @@ + + +# Code Review Reference - REVIEW_API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service setup, generic scope conflicts, loop exhaustion, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only the linked SDD/Milestone lock decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-20 +task=m-runner-proto-socket-transport-hardening/01_socket_lifecycle, plan=1, tag=REVIEW_API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `session-lifecycle`: runner id reconciliation, duplicate connection close, disconnect/timeout, reconnect 후보, heartbeat 실패 처리의 socket session lifecycle을 정리한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S02`: task=`session-lifecycle`; evidence=`Go/Dart unit 또는 smoke가 server-assigned runner id, duplicate connection close, disconnect/timeout, heartbeat failure를 검증한다.` +- Completion mode: spec-check-on-pass + +## Archive Evidence Snapshot + +- Current archived plan: `agent-task/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_cloud_G07_0.log` +- Current archived review: `agent-task/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_cloud_G07_0.log` +- Verdict: FAIL +- Required summary: + - `services/core/internal/runnersocket/server.go:350`의 `setClient`가 `s.mu`를 잡은 상태에서 old `TcpClient.Close()`를 호출한다. production client에는 `attachClientHandlers`의 disconnect listener가 붙어 있고, proto-socket `Close()`는 listener를 동기 호출하므로 duplicate registration에서 `setClient` -> `old.Close()` -> listener -> `removeClient()` -> 같은 `s.mu` 재획득 경로로 교착될 수 있다. +- Affected files: + - `services/core/internal/runnersocket/server.go` + - `services/core/internal/runnersocket/server_test.go` +- Prior verification evidence: + - `cd services/core && go test -count=1 ./...` 통과 + - `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` 통과 + - `cd apps/runner && dart analyze` 통과 +- Verification trust note: + - 기존 Go test는 listener가 없는 `pipeClient`만 `setClient`에 넣어 production disconnect listener 재진입을 검증하지 못했다. +- Roadmap/spec carryover: + - Roadmap Task `session-lifecycle` 및 SDD `S02`를 계속 완료 대상으로 유지한다. +- Allowed narrow reread: + - 필요하면 위 current archived plan/review log만 좁게 다시 읽는다. `agent-task/archive/**`는 읽지 않는다. + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-local-G06.md` -> `code_review_local_G06_N.log`, `PLAN-local-G06.md` -> `plan_local_G06_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_API-1] duplicate socket replacement deadlock 제거 | [x] | + +## 구현 체크리스트 + +- [x] [REVIEW_API-1] duplicate socket replacement deadlock을 제거하고 production listener 경로 테스트를 추가한다. 검증: `cd services/core && go test -count=1 ./internal/runnersocket` +- [x] 전체 Go 회귀를 실행한다. 검증: `cd services/core && go test -count=1 ./...` +- [x] 전체 agent 검증을 재실행한다. 검증: `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` 및 `cd apps/runner && dart analyze` +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G06_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_local_G06_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/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/`를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [x] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [x] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-runner-proto-socket-transport-hardening/`를 제거하거나, 남은 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가 연결된 SDD/Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +없음. PLAN의 방향 예시 그대로 `setClient`의 lock 해제 후 Close 구조를 채택하고, production listener를 가진 client로 재진입 테스트를 추가했다. + +## 주요 설계 결정 + +1. **lock 해제 후 Close 순서**: `s.mu.Unlock()` 직후 `old.Close()` 호출. new client가 이미 map에 있으므로 old listener가 실행될 때 `removeClient("runner-E", old)`는 map 포인터 불일치로 false를 반환해 `registry.Disconnect`를 건너뛴다. 기존 stale disconnect guard와 결합해 safe. + +2. **테스트 방식**: Go test의 기본 `-timeout` (30s)으로 deadlock을 감지한다. 명시적 `time.After` 타임아웃 없이 `setClient` 호출 완료 후 상태를 assert했다. listener가 재진입하면 `setClient` 호출 자체가 blocking해 테스트가 자연히 timeout된다. PLAN이 "타임아웃으로 감지하지 않고 positive assert"를 요구했으므로 이 방식을 채택했다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- `setClient`가 `s.mu`를 잡은 채 `TcpClient.Close()`를 호출하지 않는지 확인한다. +- duplicate replacement 테스트가 production disconnect listener 재진입 경로를 실제로 포함하는지 확인한다. +- old client disconnect listener가 current client와 registry online 상태를 덮지 않는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: + +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. + +### REVIEW_API-1 중간 검증 +```bash +$ cd services/core && go test -count=1 -timeout=30s ./internal/runnersocket +ok github.com/toki/oto/services/core/internal/runnersocket 0.047s +``` + +### 최종 검증 +```bash +$ cd services/core && go test -count=1 ./... +? github.com/toki/oto/services/core/cmd/oto-core [no test files] +ok github.com/toki/oto/services/core/internal/cicdstate 0.002s +ok github.com/toki/oto/services/core/internal/httpserver 0.068s +ok github.com/toki/oto/services/core/internal/runnerregistry 0.004s +ok github.com/toki/oto/services/core/internal/runnersocket 0.053s +? github.com/toki/oto/services/core/oto [no test files] +$ cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +00:12 +50: All tests passed! +$ cd apps/runner && dart analyze +Analyzing runner... +No issues found! +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 코드리뷰 결과 + +- 종합 판정: PASS +- 차원별 평가: + - correctness: Pass + - completeness: Pass + - test coverage: Pass + - API contract: Pass + - code quality: Pass + - plan deviation: Pass + - verification trust: Pass + - spec conformance: Pass +- 발견된 문제: 없음 +- 다음 단계: + - PASS: `complete.log`를 작성하고 task directory를 archive로 이동한다. diff --git a/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/complete.log b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/complete.log new file mode 100644 index 0000000..cc3d58e --- /dev/null +++ b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/complete.log @@ -0,0 +1,51 @@ +# Complete - m-runner-proto-socket-transport-hardening/01_socket_lifecycle + +## 완료 일시 + +2026-06-20 + +## 요약 + +Runner socket session lifecycle 보강을 2회 리뷰 루프로 완료했다. 최종 판정은 PASS다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_cloud_G07_0.log` | `code_review_cloud_G07_0.log` | FAIL | duplicate runner client replacement에서 old client close가 server mutex 재진입 deadlock을 만들 수 있어 follow-up 필요 | +| `plan_local_G06_1.log` | `code_review_local_G06_1.log` | PASS | old client close를 mutex 밖으로 이동하고 production-style disconnect listener 재진입 테스트로 고정 | + +## 구현/정리 내용 + +- `services/core/internal/runnersocket.Server.setClient`가 새 client를 map에 저장하고 lock을 해제한 뒤 old client를 닫도록 정리했다. +- duplicate replacement 중 old client disconnect listener가 `removeClient`로 재진입해도 current client와 registry 상태를 덮지 않는 테스트를 추가했다. +- Go/Dart socket session lifecycle 검증으로 SDD `S02` evidence를 충족했다. + +## 최종 검증 + +- `cd services/core && go test -count=1 ./internal/runnersocket` - PASS; `ok github.com/toki/oto/services/core/internal/runnersocket 0.048s` +- `cd services/core && go test -count=1 ./...` - PASS; core packages all passed, including `internal/runnersocket` +- `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` - PASS; `00:07 +50: All tests passed!` +- `cd apps/runner && dart analyze` - PASS; `No issues found!` + +## Roadmap Completion + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Completed task ids: + - `session-lifecycle`: PASS; evidence=`agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_local_G06_1.log`, `agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_local_G06_1.log`; verification=`cd services/core && go test -count=1 ./internal/runnersocket`, `cd services/core && go test -count=1 ./...`, `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart`, `cd apps/runner && dart analyze` +- Not completed task ids: 없음 + +## Spec Completion + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Completed scenario ids: + - `S02`: PASS; task=`session-lifecycle`; evidence=`agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_local_G06_1.log`, `agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_local_G06_1.log`; verification=`cd services/core && go test -count=1 ./internal/runnersocket`, `cd services/core && go test -count=1 ./...`, `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart`, `cd apps/runner && dart analyze` +- Not completed scenario ids: 없음 + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_cloud_G07_0.log b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_cloud_G07_0.log new file mode 100644 index 0000000..376325f --- /dev/null +++ b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_cloud_G07_0.log @@ -0,0 +1,187 @@ + + +# Plan - API + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-cloud-G07.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것까지가 구현입니다. 검증을 실행하고 실제 stdout/stderr를 붙인 뒤 active 파일을 그대로 두고 review ready로 보고하세요. 선택된 SDD 결정 또는 Milestone `구현 잠금 > 결정 필요` 항목이 막으면 review stub의 `사용자 리뷰 요청` 섹션을 채우고 멈추며, 직접 사용자에게 질문하거나 `USER_REVIEW.md`, `complete.log`, archive를 만들지 않습니다. 환경/secret/서비스 문제나 증거 공백은 일반 follow-up 대상으로 기록합니다. + +## 배경 + +현 Milestone은 기본 runner transport를 proto-socket 장기 세션으로 고정한다. 서버는 register, heartbeat, duplicate client close, disconnect listener를 갖고 있지만 `runnersocket.Server` 자체 lifecycle 테스트가 없다. 이 작업은 후속 dispatch/action/smoke 작업이 의존할 runner id reconciliation과 disconnect/timeout 기준을 먼저 안정화한다. + +## 사용자 리뷰 요청 흐름 + +사용자 리뷰 요청은 선택된 SDD 결정 또는 선택된 Milestone lock 결정이 실구현을 차단할 때만 active `CODE_REVIEW-cloud-G07.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 구현 중 직접 사용자 프롬프트, 채팅 선택지, `request_user_input`, `USER_REVIEW.md` 생성은 금지이며, code-review가 검증과 파일 생성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `session-lifecycle`: runner id reconciliation, duplicate connection close, disconnect/timeout, reconnect 후보, heartbeat 실패 처리의 socket session lifecycle을 정리한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S02`: task=`session-lifecycle`; evidence=`Go/Dart unit 또는 smoke가 server-assigned runner id, duplicate connection close, disconnect/timeout, heartbeat failure를 검증한다.` +- Completion mode: spec-check-on-pass + +## 분석 결과 + +### 읽은 파일 + +- `agent-test/local/rules.md` +- `agent-test/local/agent-smoke.md` +- `agent-test/local/framework-smoke.md` +- `agent-ops/rules/project/domain/agent/rules.md` +- `agent-ops/rules/project/domain/framework/rules.md` +- `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- `services/core/internal/runnersocket/server.go` +- `services/core/internal/runnerregistry/registry.go` +- `services/core/internal/runnerregistry/registry_test.go` +- `services/core/internal/httpserver/server.go` +- `services/core/internal/httpserver/routes.go` +- `services/core/internal/httpserver/server_test.go` +- `services/core/internal/cicdstate/store.go` +- `services/core/internal/cicdstate/store_test.go` +- `apps/runner/lib/oto/agent/registration_client.dart` +- `apps/runner/lib/oto/agent/agent_runner.dart` +- `apps/runner/lib/oto/agent/agent_config.dart` +- `apps/runner/lib/oto/agent/oto_server_job_client.dart` +- `apps/runner/test/oto_agent_registration_test.dart` +- `apps/runner/test/oto_server_connection_smoke_test.dart` +- `proto/oto/runner.proto` +- `Makefile` +- `apps/runner/pubspec.yaml` +- `services/core/go.mod` + +### 테스트 환경 규칙 + +`test_env=local`을 적용했다. `agent-test/local/rules.md`는 원격 runner 기준이며 agent 변경은 `agent-test/local/agent-smoke.md`의 `cd apps/runner && dart analyze`, agent 관련 `dart test`, `cd services/core && go test ./...`를 요구한다. `proto/**` 변경은 `framework-smoke`의 `make proto`가 필요하지만, 이 plan은 proto 파일을 바꾸지 않는다. Go state/lifecycle 검증은 fresh 실행이 중요하므로 `go test -count=1 ./...`를 사용한다. + +### 테스트 커버리지 공백 + +- server socket duplicate connection close: `services/core/internal/runnersocket/server.go`에는 `setClient`가 old client를 닫지만 전용 테스트가 없다. +- socket disconnect가 current client에만 `registry.Disconnect`를 적용하는지: disconnect listener는 있으나 stale client 경합 테스트가 없다. +- Dart socket heartbeat failure 처리: `_sendHeartbeat`는 오류를 무시하지만 heartbeat 실패 후 close/stream 정리 테스트가 없다. + +### 심볼 참조 + +renamed/removed symbol 없음. `OtoServerPushJobSession`, `OtoServerSocketRegistrationClient`, `DispatchQueuedJobs`, `CancelExecution` 참조는 `rg --sort path`로 확인했다. + +### 분할 판단 + +split decision policy를 먼저 적용했다. 공유 task group은 `agent-task/m-runner-proto-socket-transport-hardening/`이다. `01_socket_lifecycle`은 선행 작업이 없는 기반 작업이다. 후속 `02+01_dispatch_state`, `03+01,02_runner_actions`, `04+01_compat_boundary`, `05+02,03,04_socket_smoke`가 이 작업의 `complete.log`에 의존한다. + +### 범위 결정 근거 + +이 plan은 runner socket session lifecycle만 다룬다. queued job dispatch, invalid RunRequest, cancel/status/self-update, HTTP compatibility test renaming, smoke 기준 문서 갱신은 후속 split plan에서 다룬다. `proto/oto/runner.proto`는 현재 register/heartbeat/cancel/run/report 메시지가 충분하므로 변경하지 않는다. + +### 빌드 등급 + +`cloud-G07`: long-running socket/session behavior와 Go/Dart 양쪽 검증이 걸려 있고, 로컬 단위 테스트만으로 잘못된 구현이 통과할 위험이 있다. + +## 구현 체크리스트 + +- [ ] [API-1] 서버 socket lifecycle 테스트와 필요한 보강을 구현한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] [API-2] Dart socket session heartbeat/close behavior 테스트와 필요한 보강을 구현한다. 검증: `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` +- [ ] 전체 agent 검증을 실행한다. 검증: `cd apps/runner && dart analyze` +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [API-1] 서버 socket lifecycle 테스트와 보강 + +문제: [server.go](/config/workspace/oto/services/core/internal/runnersocket/server.go:136)의 register handler는 accepted runner id를 client map에 저장하고 [server.go](/config/workspace/oto/services/core/internal/runnersocket/server.go:187)의 disconnect listener가 registry를 disconnected로 바꾼다. [server.go](/config/workspace/oto/services/core/internal/runnersocket/server.go:351)의 `setClient`는 duplicate client를 닫지만 stale client disconnect가 current session 상태를 덮지 않는지 검증이 없다. + +해결 방법: + +```go +// before: services/core/internal/runnersocket/server.go:351 +func (s *Server) setClient(runnerID string, client *protoSocket.TcpClient) { + s.mu.Lock() + defer s.mu.Unlock() + if old := s.clients[runnerID]; old != nil && old != client { + _ = old.Close() + } + s.clients[runnerID] = client +} +``` + +`services/core/internal/runnersocket/server_test.go`를 추가한다. real proto-socket TCP client를 쓰거나 같은 package test helper로 `setClient/removeClient/connectedRunnerIDs`와 registry transition을 검증한다. stale client disconnect가 current client를 삭제하지 않는 현재 `removeClient` guard를 테스트로 고정하고, 필요하면 disconnect listener에서 current client 확인 후 `registry.Disconnect`하도록 보강한다. + +수정 파일 및 체크리스트: + +- [ ] `services/core/internal/runnersocket/server_test.go` 추가 +- [ ] stale client disconnect가 current client를 지우지 않는 테스트 추가 +- [ ] duplicate connection close 또는 replacement behavior 테스트 추가 +- [ ] heartbeat unknown/terminal response가 registry state와 일치하는지 테스트 추가 +- [ ] 필요 시 `services/core/internal/runnersocket/server.go` 수정 + +테스트 작성: 작성한다. 테스트 이름 후보: `TestServerReplacesDuplicateRunnerClient`, `TestServerIgnoresStaleClientDisconnect`, `TestServerHeartbeatUsesRegisteredRunnerID`. + +중간 검증: + +```bash +cd services/core && go test -count=1 ./... +``` + +기대 결과: 모든 Go package 통과. + +### [API-2] Dart socket session heartbeat/close 테스트와 보강 + +문제: [registration_client.dart](/config/workspace/oto/apps/runner/lib/oto/agent/registration_client.dart:344)는 pushed `RunRequest` listener를 열고, [registration_client.dart](/config/workspace/oto/apps/runner/lib/oto/agent/registration_client.dart:350)는 heartbeat timer를 시작한다. [registration_client.dart](/config/workspace/oto/apps/runner/lib/oto/agent/registration_client.dart:372)는 close에서 timer와 stream/socket을 닫지만 socket session 전용 test가 부족하다. + +해결 방법: + +```dart +// before: apps/runner/lib/oto/agent/registration_client.dart:358 +Future _sendHeartbeat() async { + if (_closed) return; + try { + await _client.sendRequest(...); + } catch (_) { + // Ignore background heartbeat errors; proto-socket heartbeat owns liveness. + } +} +``` + +`apps/runner/test/oto_agent_registration_test.dart`에 socket registration/session 전용 group을 보강한다. 기존 HTTP session tests는 compatibility 경로로 유지하고, socket session은 fake or loopback proto-socket으로 first heartbeat, close cancels future heartbeat, runRequests stream close를 검증한다. 직접 fake가 어려우면 `oto_server_connection_smoke_test.dart`의 real Go server helper를 재사용하되 test name에 socket을 명시한다. + +수정 파일 및 체크리스트: + +- [ ] `apps/runner/test/oto_agent_registration_test.dart` 또는 `apps/runner/test/oto_server_connection_smoke_test.dart`에 socket session lifecycle test 추가 +- [ ] heartbeat failure가 process를 죽이지 않고 close가 정리되는지 검증 +- [ ] `AgentSession.close()` 후 runRequests stream이 닫히는지 검증 +- [ ] 필요 시 `apps/runner/lib/oto/agent/registration_client.dart` 수정 + +테스트 작성: 작성한다. 테스트 이름 후보: `OtoServerSocketRegistrationClient sends heartbeat and closes pushed session`. + +중간 검증: + +```bash +cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +``` + +기대 결과: 두 테스트 파일 통과. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `services/core/internal/runnersocket/server.go` | API-1 | +| `services/core/internal/runnersocket/server_test.go` | API-1 | +| `apps/runner/lib/oto/agent/registration_client.dart` | API-2 | +| `apps/runner/test/oto_agent_registration_test.dart` | API-2 | +| `apps/runner/test/oto_server_connection_smoke_test.dart` | API-2 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./... +cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +cd apps/runner && dart analyze +``` + +기대 결과: Go tests 통과, Dart agent tests 통과, analyzer issue 없음. 모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_local_G06_1.log b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_local_G06_1.log new file mode 100644 index 0000000..d7a6a12 --- /dev/null +++ b/agent-task/archive/2026/06/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_local_G06_1.log @@ -0,0 +1,116 @@ + + +# Plan - REVIEW_API + +## 이 파일을 읽는 구현 에이전트에게 + +이 plan은 직전 code-review FAIL의 Required 1건만 해결한다. `CODE_REVIEW-local-G06.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것까지가 구현이다. 구현 중 직접 사용자에게 질문하거나 `request_user_input`, `USER_REVIEW.md`, `complete.log`, archive 생성을 하지 않는다. 선택된 SDD 결정 또는 Milestone `구현 잠금 > 결정 필요` 항목이 막을 때만 review stub의 `사용자 리뷰 요청`을 채운다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `session-lifecycle`: runner id reconciliation, duplicate connection close, disconnect/timeout, reconnect 후보, heartbeat 실패 처리의 socket session lifecycle을 정리한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S02`: task=`session-lifecycle`; evidence=`Go/Dart unit 또는 smoke가 server-assigned runner id, duplicate connection close, disconnect/timeout, heartbeat failure를 검증한다.` +- Completion mode: spec-check-on-pass + +## Archive Evidence Snapshot + +- Current archived plan: `agent-task/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/plan_cloud_G07_0.log` +- Current archived review: `agent-task/m-runner-proto-socket-transport-hardening/01_socket_lifecycle/code_review_cloud_G07_0.log` +- Verdict: FAIL +- Required summary: + - `services/core/internal/runnersocket/server.go:350`의 `setClient`가 `s.mu`를 잡은 상태에서 old `TcpClient.Close()`를 호출한다. production client에는 `attachClientHandlers`의 disconnect listener가 붙어 있고, proto-socket `Close()`는 listener를 동기 호출하므로 duplicate registration에서 `setClient` -> `old.Close()` -> listener -> `removeClient()` -> 같은 `s.mu` 재획득 경로로 교착될 수 있다. +- Affected files: + - `services/core/internal/runnersocket/server.go` + - `services/core/internal/runnersocket/server_test.go` +- Prior verification evidence: + - `cd services/core && go test -count=1 ./...` 통과 + - `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` 통과 + - `cd apps/runner && dart analyze` 통과 +- Verification trust note: + - 기존 Go test는 listener가 없는 `pipeClient`만 `setClient`에 넣어 production disconnect listener 재진입을 검증하지 못했다. +- Roadmap/spec carryover: + - Roadmap Task `session-lifecycle` 및 SDD `S02`를 계속 완료 대상으로 유지한다. +- Allowed narrow reread: + - 필요하면 위 current archived plan/review log만 좁게 다시 읽는다. `agent-task/archive/**`는 읽지 않는다. + +## 배경 + +직전 구현은 stale disconnect guard와 Dart close/heartbeat smoke를 추가했고 모든 명령은 통과했다. 하지만 duplicate runner connection을 교체하는 실제 production 경로에서는 old client에 disconnect listener가 등록되어 있으므로, mutex 안에서 old client를 닫으면 listener가 같은 mutex를 다시 잡으려 해 멈출 수 있다. + +## 범위 결정 근거 + +후속 범위는 서버 socket duplicate replacement의 교착 제거와 테스트 보강만 포함한다. Dart client, dispatch 상태 전이, cancel/status/self-update, compatibility boundary, smoke 문서 갱신은 이 plan에서 수정하지 않는다. + +## 빌드 등급 + +`local-G06`: 문제와 수정 범위가 `services/core/internal/runnersocket` 안의 mutex/close 순서와 단위 테스트로 좁고, 결정적 Go 테스트로 검증 가능하다. + +## 구현 체크리스트 + +- [ ] [REVIEW_API-1] duplicate socket replacement deadlock을 제거하고 production listener 경로 테스트를 추가한다. 검증: `cd services/core && go test -count=1 ./internal/runnersocket` +- [ ] 전체 Go 회귀를 실행한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] 전체 agent 검증을 재실행한다. 검증: `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` 및 `cd apps/runner && dart analyze` +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [REVIEW_API-1] duplicate socket replacement deadlock 제거 + +문제: `services/core/internal/runnersocket/server.go:350`의 `setClient`는 `s.mu`를 잡은 상태에서 old client를 닫는다. proto-socket `Close()`는 disconnect listener를 동기 호출하고, production listener는 `s.removeClient()`로 같은 mutex를 다시 잡는다. 기존 테스트는 listener 없는 `pipeClient`를 사용해 이 재진입 경로를 놓쳤다. + +해결 방법: + +```go +// 방향 예시 +func (s *Server) setClient(runnerID string, client *protoSocket.TcpClient) { + var old *protoSocket.TcpClient + s.mu.Lock() + old = s.clients[runnerID] + s.clients[runnerID] = client + s.mu.Unlock() + + if old != nil && old != client { + _ = old.Close() + } +} +``` + +새 client를 먼저 map에 저장하고 lock을 푼 뒤 old client를 닫는다. 그러면 old listener가 실행되어도 `removeClient(runnerID, old)`는 false를 반환하고 current client와 registry online 상태를 유지할 수 있다. + +수정 파일 및 체크리스트: + +- [ ] `services/core/internal/runnersocket/server.go`에서 old client close를 mutex 밖으로 이동한다. +- [ ] `services/core/internal/runnersocket/server_test.go`에 `attachClientHandlers` 또는 동일한 production listener 재진입을 가진 client로 duplicate replacement가 완료되는 테스트를 추가한다. +- [ ] 테스트는 deadlock을 타임아웃으로 감지하지 않고, `setClient` 호출 완료와 current client 유지, stale disconnect skip을 명확히 assert한다. + +중간 검증: + +```bash +cd services/core && go test -count=1 ./internal/runnersocket +``` + +기대 결과: runnersocket package 통과. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `services/core/internal/runnersocket/server.go` | REVIEW_API-1 | +| `services/core/internal/runnersocket/server_test.go` | REVIEW_API-1 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./internal/runnersocket +cd services/core && go test -count=1 ./... +cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +cd apps/runner && dart analyze +``` + +기대 결과: 모든 명령 통과. `CODE_REVIEW-local-G06.md`의 구현 에이전트 소유 섹션에 실제 stdout/stderr를 붙인다. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/CODE_REVIEW-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/CODE_REVIEW-cloud-G07.md new file mode 100644 index 0000000..9512a2b --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/CODE_REVIEW-cloud-G07.md @@ -0,0 +1,137 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service blockers, generic scope changes, repeated failures, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only SDD/Milestone lock decisions in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-20 +task=m-runner-proto-socket-transport-hardening/02+01_dispatch_state, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `dispatch-state`: queued job dispatch, active execution gate, invalid RunRequest failure, terminal state transition을 socket transport 기준으로 검증한다. +- Completion mode: check-on-pass +- Split dependency: + - `01_socket_lifecycle` PASS complete evidence가 있어야 구현을 시작한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S03`: task=`dispatch-state`; evidence=`Socket RunRequest dispatch와 terminal report가 queued/running/completed/failed/cancelled 상태 전이를 검증한다.` +- Completion mode: spec-check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G07.md` -> `code_review_cloud_G07_N.log`, `PLAN-cloud-G07.md` -> `plan_cloud_G07_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/`로 이동한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] socket queued dispatch와 invalid RunRequest failure | [ ] | +| [API-2] socket terminal report와 active execution gate | [ ] | + +## 구현 체크리스트 + +- [ ] 선행 `01_socket_lifecycle` PASS complete evidence를 확인한다. +- [ ] [API-1] socket queued dispatch와 invalid RunRequest failure 테스트/보강을 구현한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] [API-2] socket terminal report와 active execution gate 테스트/보강을 구현한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] runner socket smoke 관련 회귀를 확인한다. 검증: `cd apps/runner && dart test test/oto_server_connection_smoke_test.dart` +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G07_N.log`로 아카이브한다. +- [ ] active `PLAN-*-G??.md`를 `plan_cloud_G07_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/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/`를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-runner-proto-socket-transport-hardening/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active plan/review 파일 또는 follow-up plan을 작성하고 `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`를 남기지 않는다. + +## 계획 대비 변경 사항 + +_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ + +## 주요 설계 결정 + +_구현 에이전트가 주요 설계 결정 사항을 기록한다._ + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- queued execution이 socket dispatch에서 정확히 한 번 running으로 claim되는지 확인한다. +- invalid or failed `RunRequest` send가 execution을 어중간한 running 상태로 남기지 않는지 확인한다. +- terminal report가 ownership/state guard를 우회하지 않는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### API-1 중간 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +``` + +### API-2 중간 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +``` + +### 최종 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +$ cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +(output) +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/PLAN-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/PLAN-cloud-G07.md new file mode 100644 index 0000000..4748bf8 --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/02+01_dispatch_state/PLAN-cloud-G07.md @@ -0,0 +1,175 @@ + + +# Plan - API + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-cloud-G07.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것까지가 구현입니다. 검증을 실행하고 실제 stdout/stderr를 붙인 뒤 active 파일을 그대로 두고 review ready로 보고하세요. 선택된 SDD 결정 또는 Milestone `구현 잠금 > 결정 필요` 항목이 막으면 review stub의 `사용자 리뷰 요청` 섹션을 채우고 멈추며, 직접 사용자에게 질문하거나 `USER_REVIEW.md`, `complete.log`, archive를 만들지 않습니다. 환경/secret/서비스 문제나 증거 공백은 일반 follow-up 대상으로 기록합니다. + +## 배경 + +현 Milestone은 runner 기본 transport를 proto-socket push/session 기준으로 고정한다. `DispatchQueuedJobs`는 queued execution을 claim한 뒤 socket runner에게 `RunRequest`를 보내고, runner는 terminal report를 socket으로 돌려준다. 이 작업은 queued job dispatch와 execution state transition을 socket-first 경로에서 검증해 HTTP claim/polling 잔존 동작과 섞이지 않도록 한다. + +## 사용자 리뷰 요청 흐름 + +사용자 리뷰 요청은 선택된 SDD 결정 또는 선택된 Milestone lock 결정이 실구현을 차단할 때만 active `CODE_REVIEW-cloud-G07.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 구현 중 직접 사용자 프롬프트, 채팅 선택지, `request_user_input`, `USER_REVIEW.md` 생성은 금지이며, code-review가 검증과 파일 생성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `dispatch-state`: queued job dispatch, active execution gate, invalid RunRequest failure, terminal state transition을 socket transport 기준으로 검증한다. +- Completion mode: check-on-pass +- Split dependency: + - `01_socket_lifecycle` PASS complete evidence가 있어야 구현을 시작한다. 미충족이면 코드 변경 없이 dependency pending으로 보고한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S03`: task=`dispatch-state`; evidence=`Socket RunRequest dispatch와 terminal report가 queued/running/completed/failed/cancelled 상태 전이를 검증한다.` +- Completion mode: spec-check-on-pass + +## 분석 결과 + +### 읽은 파일 + +- `agent-test/local/rules.md` +- `agent-test/local/agent-smoke.md` +- `agent-test/local/framework-smoke.md` +- `agent-ops/rules/project/domain/agent/rules.md` +- `agent-ops/rules/project/domain/framework/rules.md` +- `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- `services/core/internal/runnersocket/server.go` +- `services/core/internal/httpserver/server_test.go` +- `services/core/internal/cicdstate/store.go` +- `services/core/internal/cicdstate/store_test.go` +- `apps/runner/lib/oto/agent/registration_client.dart` +- `apps/runner/lib/oto/agent/agent_runner.dart` +- `apps/runner/test/oto_server_connection_smoke_test.dart` +- `proto/oto/runner.proto` +- `Makefile` + +### 테스트 환경 규칙 + +`test_env=local`을 적용했다. `agent-test/local/rules.md`는 agent 변경 시 `cd apps/runner && dart analyze`, 관련 `dart test`, `cd services/core && go test ./...`를 요구한다. 이 plan은 proto schema 변경 없이 socket dispatch/state test와 필요한 server/runner 보강만 다룬다. + +### 테스트 커버리지 공백 + +- socket queued dispatch가 `claimNextQueuedJob`을 통해 하나의 queued execution만 running으로 전환하는지 직접 검증이 부족하다. +- invalid `RunRequest` 전송 실패가 execution을 failed로 마감하는지 socket server 단위 테스트가 없다. +- socket terminal report가 running 상태에서 completed/failed/cancelled 계열로만 전이하는지 HTTP report tests와 분리된 증거가 부족하다. + +### 심볼 참조 + +`runnersocket.Server.DispatchQueuedJobs`, `claimNextQueuedJob`, `ReportExecutionResult`, `proto.RunRequest`, `ReportRunResultRequest`, `OtoServerPushJobSession`을 중심으로 확인했다. removed/renamed symbol 없음. + +### 분할 판단 + +이 작업은 `01_socket_lifecycle`의 session 안정화 결과에 의존한다. 후속 `03+01,02_runner_actions`와 `05+02,03,04_socket_smoke`가 이 작업의 PASS evidence에 의존한다. + +### 범위 결정 근거 + +이 plan은 queued execution dispatch와 state transition만 다룬다. cancel/status/self-update operator action 방향은 `03+01,02_runner_actions`, HTTP compatibility 경계는 `04+01_compat_boundary`, smoke 문서/명령 정리는 `05+02,03,04_socket_smoke`에서 다룬다. + +### 빌드 등급 + +`cloud-G07`: state transition은 Go server store와 socket transport, Dart runner session이 함께 얽히며 회귀 영향이 커서 full local Go test와 targeted Dart smoke가 필요하다. + +## 구현 체크리스트 + +- [ ] 선행 `01_socket_lifecycle` PASS complete evidence를 확인한다. +- [ ] [API-1] socket queued dispatch와 invalid RunRequest failure 테스트/보강을 구현한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] [API-2] socket terminal report와 active execution gate 테스트/보강을 구현한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] runner socket smoke 관련 회귀를 확인한다. 검증: `cd apps/runner && dart test test/oto_server_connection_smoke_test.dart` +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [API-1] socket queued dispatch와 invalid RunRequest failure + +문제: [server.go](/config/workspace/oto/services/core/internal/runnersocket/server.go:75)의 `DispatchQueuedJobs`는 connected runner 목록을 순회하고 [server.go](/config/workspace/oto/services/core/internal/runnersocket/server.go:270)의 `claimNextQueuedJob`으로 queued execution을 running으로 바꾼다. 다만 socket dispatch 단위에서 "한 queued job만 claim", "runner 전송 실패 시 failed"가 고정되어 있지 않다. + +해결 방법: + +```go +// before: services/core/internal/runnersocket/server.go:75 +func (s *Server) DispatchQueuedJobs(ctx context.Context) { + for _, runnerID := range s.connectedRunnerIDs() { + ... + execution, job, ok, err := s.claimNextQueuedJob(ctx, runnerID) + ... + } +} +``` + +`services/core/internal/runnersocket/server_test.go`에 dispatch 전용 tests를 추가한다. fake or loopback proto-socket client로 `RunRequest` 수신을 확인하고, 전송 오류/invalid request fixture에서는 `FailExecution` 또는 동일 정책의 failed transition을 검증한다. HTTP claim handler tests와 assertion 이름을 분리해 socket dispatch evidence로 읽히게 한다. + +수정 파일 및 체크리스트: + +- [ ] `services/core/internal/runnersocket/server_test.go`에 queued dispatch test 추가 +- [ ] dispatch가 runner id와 queued execution filter를 지키는지 검증 +- [ ] send failure 또는 invalid request가 execution을 failed로 마감하는지 검증 +- [ ] 필요 시 `services/core/internal/runnersocket/server.go` 보강 + +테스트 작성: 작성한다. 테스트 이름 후보: `TestServerDispatchQueuedJobOverSocket`, `TestServerFailsExecutionWhenRunRequestCannotBeSent`. + +중간 검증: + +```bash +cd services/core && go test -count=1 ./... +``` + +기대 결과: 모든 Go package 통과. + +### [API-2] socket terminal report와 active execution gate + +문제: [server.go](/config/workspace/oto/services/core/internal/runnersocket/server.go:197)의 report handler는 runner terminal report를 받아 `ReportExecutionResult`로 넘긴다. [store.go](/config/workspace/oto/services/core/internal/cicdstate/store.go:262)의 state transition guard가 있으나 socket report path에서 active execution과 runner ownership이 어긋난 경우의 test evidence가 부족하다. + +해결 방법: + +```go +// before: services/core/internal/runnersocket/server.go:197 +s.handlers.Register(func(ctx context.Context, req *pb.ReportRunResultRequest) (*pb.ReportRunResultResponse, error) { + err := s.state.ReportExecutionResult(ctx, req.ExecutionId, cicdstate.ExecutionResult{...}) + ... +}) +``` + +socket report handler tests를 추가해 running execution만 terminal report를 받을 수 있는지, completed/failed report가 terminal state와 timestamps/log fields를 보존하는지 확인한다. cancelled 상태와 terminal report가 충돌하는 경우에는 현재 store 정책을 테스트로 고정하거나, SDD의 cancelled evidence가 필요하면 store guard를 보강한다. + +수정 파일 및 체크리스트: + +- [ ] socket report success/completed test 추가 +- [ ] socket report failure test 추가 +- [ ] wrong runner 또는 non-running execution report guard test 추가 +- [ ] cancelled state와 terminal report 충돌 정책을 테스트로 고정 +- [ ] 필요 시 `services/core/internal/cicdstate/store.go` 보강 + +테스트 작성: 작성한다. 테스트 이름 후보: `TestServerReportsRunResultOverSocket`, `TestServerRejectsTerminalReportForInactiveExecution`. + +중간 검증: + +```bash +cd services/core && go test -count=1 ./... +``` + +기대 결과: 모든 Go package 통과. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `services/core/internal/runnersocket/server.go` | API-1, API-2 | +| `services/core/internal/runnersocket/server_test.go` | API-1, API-2 | +| `services/core/internal/cicdstate/store.go` | API-2 | +| `services/core/internal/cicdstate/store_test.go` | API-2 | +| `apps/runner/test/oto_server_connection_smoke_test.dart` | API-1, API-2 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./... +cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +``` + +기대 결과: Go tests 통과, socket connection smoke 통과. 모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/CODE_REVIEW-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/CODE_REVIEW-cloud-G07.md new file mode 100644 index 0000000..b49b48d --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/CODE_REVIEW-cloud-G07.md @@ -0,0 +1,147 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service blockers, generic scope changes, repeated failures, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only SDD/Milestone lock decisions in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-20 +task=m-runner-proto-socket-transport-hardening/03+01,02_runner_actions, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `runner-actions`: cancel/status/self-update runner action의 socket/HTTP 방향을 정하고 취소와 terminal report 충돌을 막는다. +- Completion mode: check-on-pass +- Split dependency: + - `01_socket_lifecycle` PASS complete evidence가 있어야 구현을 시작한다. + - `02+01_dispatch_state` PASS complete evidence가 있어야 구현을 시작한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S04`: task=`runner-actions`; evidence=`cancel/status/self-update 요청 경로가 socket-first flow 또는 compatibility 제외 범위로 명시되고 검증된다.` +- Completion mode: spec-check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G07.md` -> `code_review_cloud_G07_N.log`, `PLAN-cloud-G07.md` -> `plan_cloud_G07_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/`로 이동한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] cancel/status action의 socket-first 방향과 상태 guard | [ ] | +| [API-2] runner-side cancel handling 또는 compatibility 제외 | [ ] | +| [API-3] self-update action 방향 명확화 | [ ] | + +## 구현 체크리스트 + +- [ ] 선행 `01_socket_lifecycle`, `02+01_dispatch_state` PASS complete evidence를 확인한다. +- [ ] [API-1] cancel/status action의 socket-first 방향과 상태 guard를 구현/검증한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] [API-2] runner-side cancel handling 또는 명시적 compatibility 제외를 구현/검증한다. 검증: `cd apps/runner && dart test test/oto_server_connection_smoke_test.dart` +- [ ] [API-3] self-update action을 socket-first 구현 또는 compatibility 제외 범위로 명확히 한다. 검증: `cd apps/runner && dart analyze` +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G07_N.log`로 아카이브한다. +- [ ] active `PLAN-*-G??.md`를 `plan_cloud_G07_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/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/`를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-runner-proto-socket-transport-hardening/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active plan/review 파일 또는 follow-up plan을 작성하고 `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`를 남기지 않는다. + +## 계획 대비 변경 사항 + +_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ + +## 주요 설계 결정 + +_구현 에이전트가 주요 설계 결정 사항을 기록한다._ + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- cancel endpoint와 socket cancel delivery의 관계가 테스트 또는 명시적 제외로 닫혔는지 확인한다. +- late terminal report가 cancelled state를 뒤집지 않는지 확인한다. +- self-update가 socket-first 구현이 아니라면 compatibility 제외 범위가 SDD S04 evidence와 충돌하지 않는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### API-1 중간 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +``` + +### API-2 중간 검증 +```bash +$ cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +(output) +``` + +### API-3 중간 검증 +```bash +$ cd apps/runner && dart analyze +(output) +``` + +### 최종 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +$ cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +(output) +$ cd apps/runner && dart analyze +(output) +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/PLAN-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/PLAN-cloud-G07.md new file mode 100644 index 0000000..a2cc09b --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/03+01,02_runner_actions/PLAN-cloud-G07.md @@ -0,0 +1,203 @@ + + +# Plan - API + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-cloud-G07.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것까지가 구현입니다. 검증을 실행하고 실제 stdout/stderr를 붙인 뒤 active 파일을 그대로 두고 review ready로 보고하세요. 선택된 SDD 결정 또는 Milestone `구현 잠금 > 결정 필요` 항목이 막으면 review stub의 `사용자 리뷰 요청` 섹션을 채우고 멈추며, 직접 사용자에게 질문하거나 `USER_REVIEW.md`, `complete.log`, archive를 만들지 않습니다. 환경/secret/서비스 문제나 증거 공백은 일반 follow-up 대상으로 기록합니다. + +## 배경 + +operator HTTP action 표면은 cancel/status/self-update 요청을 받지만, 현 Milestone의 기본 runner transport는 proto-socket이다. cancel은 server에서 runner socket으로 내려갈 수 있는 `CancelExecution` 경로가 있고, status는 state read 성격이 강하며, self-update는 runner-side 실행 경로가 아직 HTTP compatibility로 남아 있다. 이 작업은 세 action의 방향을 socket-first 기준으로 명확히 하고, cancel/report 충돌을 상태 전이에서 안전하게 막는다. + +## 사용자 리뷰 요청 흐름 + +사용자 리뷰 요청은 선택된 SDD 결정 또는 선택된 Milestone lock 결정이 실구현을 차단할 때만 active `CODE_REVIEW-cloud-G07.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 구현 중 직접 사용자 프롬프트, 채팅 선택지, `request_user_input`, `USER_REVIEW.md` 생성은 금지이며, code-review가 검증과 파일 생성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `runner-actions`: cancel/status/self-update runner action의 socket/HTTP 방향을 정하고 취소와 terminal report 충돌을 막는다. +- Completion mode: check-on-pass +- Split dependency: + - `01_socket_lifecycle` PASS complete evidence가 있어야 구현을 시작한다. + - `02+01_dispatch_state` PASS complete evidence가 있어야 구현을 시작한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S04`: task=`runner-actions`; evidence=`cancel/status/self-update 요청 경로가 socket-first flow 또는 compatibility 제외 범위로 명시되고 검증된다.` +- Completion mode: spec-check-on-pass + +## 분석 결과 + +### 읽은 파일 + +- `agent-test/local/rules.md` +- `agent-test/local/agent-smoke.md` +- `agent-test/local/framework-smoke.md` +- `agent-ops/rules/project/domain/agent/rules.md` +- `agent-ops/rules/project/domain/framework/rules.md` +- `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- `services/core/internal/httpserver/runner_cicd_handlers.go` +- `services/core/internal/runnersocket/server.go` +- `services/core/internal/cicdstate/store.go` +- `services/core/internal/httpserver/server_test.go` +- `apps/runner/lib/oto/agent/registration_client.dart` +- `apps/runner/lib/oto/agent/agent_runner.dart` +- `apps/runner/lib/oto/agent/oto_server_job_client.dart` +- `apps/runner/test/oto_server_connection_smoke_test.dart` +- `proto/oto/runner.proto` +- `Makefile` + +### 테스트 환경 규칙 + +`test_env=local`을 적용했다. agent/core 양쪽 action behavior가 걸리므로 Go 전체 테스트, runner socket smoke, Dart analyzer를 실행한다. proto schema를 변경하면 `make proto`와 generated code 갱신이 추가로 필요하지만, 기본 plan은 기존 `CancelRunRequest`, `SelfUpdateRequest`, status HTTP DTO 범위에서 처리한다. + +### 테스트 커버리지 공백 + +- operator cancel endpoint와 socket `CancelExecution` delivery가 하나의 흐름으로 검증되지 않는다. +- runner Dart session은 pushed `RunRequest`만 처리하며 `CancelRunRequest` listener가 없다. +- self-update request는 proto message와 HTTP client method가 있으나 socket consumer가 없어서 compatibility 제외 여부가 문서/테스트에서 불명확하다. +- cancel 이후 late terminal report가 cancelled state를 뒤집지 않는지 socket action scenario로 고정되어 있지 않다. + +### 심볼 참조 + +`CancelExecution`, `CancelExecutionHandler`, `GetExecutionStatusHandler`, `RequestRunnerSelfUpdateHandler`, `CancelRunRequest`, `SelfUpdateRequest`, `OtoServerJobClient.cancelRun`, `requestSelfUpdate`를 중심으로 확인했다. removed/renamed symbol 없음. + +### 분할 판단 + +이 작업은 session lifecycle과 dispatch/state guard가 선행되어야 의미가 있다. 후속 `05+02,03,04_socket_smoke`는 이 작업에서 확정한 action direction을 smoke pass criteria에 반영한다. + +### 범위 결정 근거 + +이 plan은 action direction과 cancel/report 충돌만 다룬다. queued dispatch의 일반 상태 전이는 `02+01_dispatch_state`, HTTP fallback 명명과 compatibility boundary는 `04+01_compat_boundary`에서 다룬다. + +### 빌드 등급 + +`cloud-G07`: action behavior가 operator HTTP endpoint, core state store, socket server, Dart runner session을 가로지른다. 반쪽 구현이 smoke에서 늦게 드러날 수 있어 full local core/agent verification이 필요하다. + +## 구현 체크리스트 + +- [ ] 선행 `01_socket_lifecycle`, `02+01_dispatch_state` PASS complete evidence를 확인한다. +- [ ] [API-1] cancel/status action의 socket-first 방향과 상태 guard를 구현/검증한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] [API-2] runner-side cancel handling 또는 명시적 compatibility 제외를 구현/검증한다. 검증: `cd apps/runner && dart test test/oto_server_connection_smoke_test.dart` +- [ ] [API-3] self-update action을 socket-first 구현 또는 compatibility 제외 범위로 명확히 한다. 검증: `cd apps/runner && dart analyze` +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [API-1] cancel/status action의 socket-first 방향과 상태 guard + +문제: [runner_cicd_handlers.go](/config/workspace/oto/services/core/internal/httpserver/runner_cicd_handlers.go:367)는 operator cancel endpoint를 제공하고, [server.go](/config/workspace/oto/services/core/internal/runnersocket/server.go:108)는 runner에게 socket cancel을 보낼 수 있다. 두 경로가 한 action flow로 묶여 있는지와 late terminal report guard가 부족하다. + +해결 방법: + +```go +// before: services/core/internal/runnersocket/server.go:108 +func (s *Server) CancelExecution(ctx context.Context, runnerID string, executionID string) error { + client := s.clientForRunner(runnerID) + ... +} +``` + +operator cancel handler가 state cancel만 수행하는지, socket runner cancel delivery까지 수행해야 하는지 현재 wiring을 확인한다. SDD S04 기준으로 socket-first가 목표이면 cancel endpoint에서 runner id를 찾아 `CancelExecution`을 호출할 수 있도록 dependency 주입을 보강한다. status endpoint는 state read action으로 명시하고 socket delivery 대상이 아님을 테스트/문서에 남긴다. + +수정 파일 및 체크리스트: + +- [ ] cancel endpoint에서 state cancel과 runner socket cancel delivery 관계를 명확히 함 +- [ ] cancel 후 late terminal report가 cancelled state를 뒤집지 않는 테스트 추가 +- [ ] status endpoint가 socket command가 아니라 state query임을 테스트 이름/설명으로 명확히 함 +- [ ] 필요 시 `services/core/internal/httpserver/server.go` 또는 handler dependency 보강 + +테스트 작성: 작성한다. 테스트 이름 후보: `TestCancelExecutionSendsSocketCancelToRunner`, `TestCancelPreventsLateTerminalReportFromOverwritingCancelledState`. + +중간 검증: + +```bash +cd services/core && go test -count=1 ./... +``` + +기대 결과: 모든 Go package 통과. + +### [API-2] runner-side cancel handling 또는 compatibility 제외 + +문제: [registration_client.dart](/config/workspace/oto/apps/runner/lib/oto/agent/registration_client.dart:418)는 `CancelRunRequest` parser를 등록하지만 [agent_runner.dart](/config/workspace/oto/apps/runner/lib/oto/agent/agent_runner.dart:108)의 socket push loop는 `RunRequest`만 처리한다. socket cancel을 실제로 보낼 경우 runner가 수신 후 active execution을 중단하거나, 현재 Milestone에서 제외한다고 명시해야 한다. + +해결 방법: + +```dart +// before: apps/runner/lib/oto/agent/agent_runner.dart:108 +await for (final runRequest in session.runRequests) { + ... +} +``` + +가능한 범위에서 push session을 typed command stream으로 확장해 `CancelRunRequest`를 처리한다. active process cancel이 runner executor 구조상 큰 변경이면, 이번 Milestone에서는 cancel delivery evidence를 core socket send까지로 제한하고 runner process cancel은 compatibility/follow-up으로 명시한다. 단, 이 제외는 SDD S04 evidence에 남겨야 하며 smoke pass criteria와 충돌하면 사용자 리뷰 요청이 아니라 follow-up plan 대상으로 기록한다. + +수정 파일 및 체크리스트: + +- [ ] runner socket client가 `CancelRunRequest`를 수신할 수 있는지 확인/테스트 +- [ ] active execution cancel handling 구현 가능성을 판단하고 구현 또는 범위 제외 기록 +- [ ] core에서 cancel send 실패 시 operator action response/state 정책을 고정 +- [ ] 필요 시 runner smoke에 cancel request 수신 검증 추가 + +테스트 작성: 작성한다. 테스트 이름 후보: `runner receives socket cancel for active execution`. + +중간 검증: + +```bash +cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +``` + +기대 결과: socket smoke 통과. + +### [API-3] self-update action 방향 명확화 + +문제: [proto/runner.proto](/config/workspace/oto/proto/oto/runner.proto:169)는 `SelfUpdateRequest`를 정의하고 [oto_server_job_client.dart](/config/workspace/oto/apps/runner/lib/oto/agent/oto_server_job_client.dart:344)는 HTTP self-update client를 제공한다. 그러나 server-side self-update socket dispatch와 runner consumer가 없다. + +해결 방법: + +이번 Milestone에서 self-update를 socket command로 구현할지, HTTP compatibility action으로 제외할지 결정하고 코드/테스트 이름/문서에 반영한다. 기본 추천은 product surface가 아직 operator action 수준이므로 self-update는 compatibility 제외로 명시하고, socket command 구현은 별도 Milestone으로 넘기는 것이다. 단, 기존 code path가 socket-first rule을 위반해 기본 path처럼 보이면 이름과 테스트를 compatibility로 정리한다. + +수정 파일 및 체크리스트: + +- [ ] self-update HTTP path가 compatibility/follow-up인지 명시 +- [ ] socket-first smoke가 self-update를 필수 socket command로 오해하지 않게 정리 +- [ ] 필요 시 roadmap SDD evidence에 excluded compatibility wording 업데이트 +- [ ] Dart analyzer 통과 확인 + +테스트 작성: 변경 범위에 따라 작성한다. 문서/테스트 이름만 바꾸면 기존 smoke와 analyzer로 검증한다. + +중간 검증: + +```bash +cd apps/runner && dart analyze +``` + +기대 결과: analyzer issue 없음. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `services/core/internal/httpserver/runner_cicd_handlers.go` | API-1, API-3 | +| `services/core/internal/httpserver/server.go` | API-1 | +| `services/core/internal/runnersocket/server.go` | API-1 | +| `services/core/internal/cicdstate/store.go` | API-1 | +| `services/core/internal/httpserver/server_test.go` | API-1, API-3 | +| `apps/runner/lib/oto/agent/agent_runner.dart` | API-2 | +| `apps/runner/lib/oto/agent/registration_client.dart` | API-2 | +| `apps/runner/lib/oto/agent/oto_server_job_client.dart` | API-3 | +| `apps/runner/test/oto_server_connection_smoke_test.dart` | API-2, API-3 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./... +cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +cd apps/runner && dart analyze +``` + +기대 결과: Go tests 통과, runner socket smoke 통과, analyzer issue 없음. 모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/CODE_REVIEW-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/CODE_REVIEW-cloud-G07.md new file mode 100644 index 0000000..750d0b9 --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/CODE_REVIEW-cloud-G07.md @@ -0,0 +1,148 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service blockers, generic scope changes, repeated failures, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only SDD/Milestone lock decisions in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-20 +task=m-runner-proto-socket-transport-hardening/04+01_compat_boundary, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `compat-boundary`: HTTP job claim/polling/report/log/artifact 경로를 compatibility fallback으로 격리하고 기본 runner path 명명에서 제외한다. +- Completion mode: check-on-pass +- Split dependency: + - `01_socket_lifecycle` PASS complete evidence가 있어야 구현을 시작한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S05`: task=`compat-boundary`; evidence=`HTTP job claim/report/log/artifact 경로가 compatibility fallback으로 격리되고 기본 smoke/문서가 socket path를 기준으로 한다.` +- Completion mode: spec-check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G07.md` -> `code_review_cloud_G07_N.log`, `PLAN-cloud-G07.md` -> `plan_cloud_G07_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/`로 이동한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] runner default path와 HTTP fallback boundary 분리 | [ ] | +| [API-2] core HTTP job endpoints compatibility evidence 정리 | [ ] | +| [API-3] grep evidence와 smoke/docs 정리 | [ ] | + +## 구현 체크리스트 + +- [ ] 선행 `01_socket_lifecycle` PASS complete evidence를 확인한다. +- [ ] [API-1] runner default path와 HTTP fallback boundary를 코드/테스트 이름에서 분리한다. 검증: `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` +- [ ] [API-2] core HTTP job endpoints가 compatibility로 유지되는지 tests/docs evidence를 정리한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] [API-3] deterministic grep evidence로 기본 smoke/docs가 socket-first인지 확인한다. 검증: `rg --sort path -n "OtoServerRegistrationClient|claimNextJob|jobs/claim|HTTP polling|compatibility|fallback|socket" apps/runner/lib apps/runner/test services/core/internal/httpserver agent-ops/rules/project/domain/agent/rules.md agent-test/local/agent-smoke.md` +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G07_N.log`로 아카이브한다. +- [ ] active `PLAN-*-G??.md`를 `plan_cloud_G07_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/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/`를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-runner-proto-socket-transport-hardening/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active plan/review 파일 또는 follow-up plan을 작성하고 `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`를 남기지 않는다. + +## 계획 대비 변경 사항 + +_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ + +## 주요 설계 결정 + +_구현 에이전트가 주요 설계 결정 사항을 기록한다._ + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- HTTP job routes가 삭제되지 않고 compatibility fallback으로 보존됐는지 확인한다. +- default runner/smoke 경로가 socket session을 기준으로 읽히는지 확인한다. +- grep evidence가 단순 키워드 나열이 아니라 fallback 경계 확인에 충분한지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### API-1 중간 검증 +```bash +$ cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +(output) +``` + +### API-2 중간 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +``` + +### API-3 중간 검증 +```bash +$ rg --sort path -n "OtoServerRegistrationClient|claimNextJob|jobs/claim|HTTP polling|compatibility|fallback|socket" apps/runner/lib apps/runner/test services/core/internal/httpserver agent-ops/rules/project/domain/agent/rules.md agent-test/local/agent-smoke.md +(output) +``` + +### 최종 검증 +```bash +$ cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +(output) +$ cd services/core && go test -count=1 ./... +(output) +$ cd apps/runner && dart analyze +(output) +$ rg --sort path -n "OtoServerRegistrationClient|claimNextJob|jobs/claim|HTTP polling|compatibility|fallback|socket" apps/runner/lib apps/runner/test services/core/internal/httpserver agent-ops/rules/project/domain/agent/rules.md agent-test/local/agent-smoke.md +(output) +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/PLAN-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/PLAN-cloud-G07.md new file mode 100644 index 0000000..31ad8a7 --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/04+01_compat_boundary/PLAN-cloud-G07.md @@ -0,0 +1,192 @@ + + +# Plan - API + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-cloud-G07.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것까지가 구현입니다. 검증을 실행하고 실제 stdout/stderr를 붙인 뒤 active 파일을 그대로 두고 review ready로 보고하세요. 선택된 SDD 결정 또는 Milestone `구현 잠금 > 결정 필요` 항목이 막으면 review stub의 `사용자 리뷰 요청` 섹션을 채우고 멈추며, 직접 사용자에게 질문하거나 `USER_REVIEW.md`, `complete.log`, archive를 만들지 않습니다. 환경/secret/서비스 문제나 증거 공백은 일반 follow-up 대상으로 기록합니다. + +## 배경 + +Milestone SDD는 runner 기본 연결을 proto-socket으로 고정하고 HTTP job claim/report/log/artifact 경로를 compatibility fallback으로 축소한다. 현재 runner는 socket registration 실패 시 HTTP session으로 fallback하고, HTTP route/tests는 여전히 기본 runner path처럼 보이는 이름과 smoke 배치를 일부 갖고 있다. 이 작업은 HTTP 경로를 제거하지 않고 경계와 이름, 검증 기준을 정리한다. + +## 사용자 리뷰 요청 흐름 + +사용자 리뷰 요청은 선택된 SDD 결정 또는 선택된 Milestone lock 결정이 실구현을 차단할 때만 active `CODE_REVIEW-cloud-G07.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 구현 중 직접 사용자 프롬프트, 채팅 선택지, `request_user_input`, `USER_REVIEW.md` 생성은 금지이며, code-review가 검증과 파일 생성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `compat-boundary`: HTTP job claim/polling/report/log/artifact 경로를 compatibility fallback으로 격리하고 기본 runner path 명명에서 제외한다. +- Completion mode: check-on-pass +- Split dependency: + - `01_socket_lifecycle` PASS complete evidence가 있어야 구현을 시작한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S05`: task=`compat-boundary`; evidence=`HTTP job claim/report/log/artifact 경로가 compatibility fallback으로 격리되고 기본 smoke/문서가 socket path를 기준으로 한다.` +- Completion mode: spec-check-on-pass + +## 분석 결과 + +### 읽은 파일 + +- `agent-test/local/rules.md` +- `agent-test/local/agent-smoke.md` +- `agent-test/local/framework-smoke.md` +- `agent-ops/rules/project/domain/agent/rules.md` +- `agent-ops/rules/project/domain/framework/rules.md` +- `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- `services/core/internal/httpserver/routes.go` +- `services/core/internal/httpserver/job_handlers.go` +- `services/core/internal/httpserver/runner_handlers.go` +- `services/core/internal/httpserver/server_test.go` +- `apps/runner/lib/oto/agent/agent_runner.dart` +- `apps/runner/lib/oto/agent/registration_client.dart` +- `apps/runner/lib/oto/agent/oto_server_job_client.dart` +- `apps/runner/test/oto_agent_registration_test.dart` +- `apps/runner/test/oto_server_connection_smoke_test.dart` +- `Makefile` + +### 테스트 환경 규칙 + +`test_env=local`을 적용했다. 이 작업은 대부분 naming, fallback boundary, tests/docs 정리지만 agent/core 경로를 건드릴 수 있으므로 Go test, Dart targeted tests, analyzer, deterministic `rg --sort path` evidence를 사용한다. + +### 테스트 커버리지 공백 + +- HTTP polling loop가 fallback임을 test names/pass criteria가 일관되게 표현하지 않는다. +- `OtoServerRegistrationClient`와 `OtoServerJobClient.claimNextJob`가 기본 path smoke처럼 보일 수 있다. +- HTTP routes는 유지되어야 하지만 compatibility route로 분류되는 evidence가 부족하다. + +### 심볼 참조 + +`OtoServerPushJobSession`, `OtoServerSocketRegistrationClient`, `OtoServerRegistrationClient`, `OtoServerJobClient.claimNextJob`, `/api/runners/jobs/claim`, `jobs/{id}/report`, `jobs/{id}/logs`, `jobs/{id}/artifacts`를 중심으로 확인했다. removed/renamed symbol 없음. + +### 분할 판단 + +이 작업은 session lifecycle이 먼저 안정화되어야 fallback naming이 의미 있다. dispatch/action 구현과 직접 충돌하지 않도록 HTTP compatibility naming과 test grouping만 좁게 다룬다. + +### 범위 결정 근거 + +HTTP job endpoints를 삭제하지 않는다. 기존 외부/legacy runner compatibility를 깨지 않고, 기본 agent path와 smoke 기준에서 socket-first를 명확히 하는 데 집중한다. + +### 빌드 등급 + +`cloud-G07`: 기능 삭제는 아니지만 route/test naming과 default path 기준이 바뀌면 runner smoke와 운영 문서 신뢰도에 영향이 있다. + +## 구현 체크리스트 + +- [ ] 선행 `01_socket_lifecycle` PASS complete evidence를 확인한다. +- [ ] [API-1] runner default path와 HTTP fallback boundary를 코드/테스트 이름에서 분리한다. 검증: `cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart` +- [ ] [API-2] core HTTP job endpoints가 compatibility로 유지되는지 tests/docs evidence를 정리한다. 검증: `cd services/core && go test -count=1 ./...` +- [ ] [API-3] deterministic grep evidence로 기본 smoke/docs가 socket-first인지 확인한다. 검증: 아래 `rg --sort path` 명령 +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [API-1] runner default path와 HTTP fallback boundary 분리 + +문제: [agent_runner.dart](/config/workspace/oto/apps/runner/lib/oto/agent/agent_runner.dart:77)는 socket session을 먼저 시도하고 실패 시 HTTP session으로 fallback한다. 그러나 [agent_runner.dart](/config/workspace/oto/apps/runner/lib/oto/agent/agent_runner.dart:135)의 HTTP polling loop와 tests가 기본 runner 동작처럼 읽힐 수 있다. + +해결 방법: + +```dart +// before: apps/runner/lib/oto/agent/agent_runner.dart:77 +final socketSession = await _tryStartSocketSession(config, registration); +if (socketSession != null) { + return _runPushLoop(socketSession); +} +return _runPollingLoop(...); +``` + +fallback behavior는 유지하되, function/test group/log wording을 compatibility fallback으로 정리한다. socket path test를 first/default smoke로 두고 HTTP polling tests는 legacy/compatibility group으로 분리한다. + +수정 파일 및 체크리스트: + +- [ ] runner test group names에서 socket default와 HTTP compatibility를 구분 +- [ ] HTTP fallback log/test wording이 default path로 읽히지 않게 정리 +- [ ] socket session 실패 시 fallback behavior는 기존대로 보존 +- [ ] 필요 시 `agent-ops/rules/project/domain/agent/rules.md` 문구와 일치하도록 테스트명 보강 + +테스트 작성: 기존 tests rename/grouping 또는 신규 assertion 작성. + +중간 검증: + +```bash +cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +``` + +기대 결과: targeted Dart tests 통과. + +### [API-2] core HTTP job endpoints compatibility evidence 정리 + +문제: [routes.go](/config/workspace/oto/services/core/internal/httpserver/routes.go:107)는 HTTP runner job routes를 계속 노출한다. 이는 legacy compatibility로 필요하지만 Milestone 기본 transport와 혼동되면 안 된다. + +해결 방법: + +HTTP job claim/report/log/artifact tests는 compatibility/fallback group으로 유지한다. route 제거는 하지 않는다. endpoint docs/comments/test names가 기본 transport처럼 보이면 compatibility wording으로 정리한다. + +수정 파일 및 체크리스트: + +- [ ] HTTP job claim/report/log/artifact tests가 compatibility fallback evidence로 읽히도록 정리 +- [ ] route behavior는 유지 +- [ ] socket dispatch tests와 HTTP claim tests가 서로 다른 책임을 갖도록 이름 분리 + +테스트 작성: 기존 Go tests 이름/grouping 또는 comments 정리. + +중간 검증: + +```bash +cd services/core && go test -count=1 ./... +``` + +기대 결과: 모든 Go package 통과. + +### [API-3] grep evidence와 smoke/docs 정리 + +문제: smoke criteria와 docs가 socket-first를 명확히 하지 않으면 후속 작업이 HTTP polling을 기본 evidence로 제출할 수 있다. + +해결 방법: + +`agent-test/local/agent-smoke.md`, domain rule, tests를 정리한 뒤 deterministic grep으로 socket-first/fallback wording을 확인한다. + +수정 파일 및 체크리스트: + +- [ ] `agent-test/local/agent-smoke.md`가 socket smoke를 기본으로 요구하는지 확인/수정 +- [ ] HTTP 관련 wording이 compatibility fallback으로 제한되는지 확인 +- [ ] `rg --sort path` 결과를 CODE_REVIEW 검증 결과에 붙임 + +중간 검증: + +```bash +rg --sort path -n "OtoServerRegistrationClient|claimNextJob|jobs/claim|HTTP polling|compatibility|fallback|socket" apps/runner/lib apps/runner/test services/core/internal/httpserver agent-ops/rules/project/domain/agent/rules.md agent-test/local/agent-smoke.md +``` + +기대 결과: HTTP client/claim/polling 언급이 compatibility/fallback 맥락으로 분리되어 보인다. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `apps/runner/lib/oto/agent/agent_runner.dart` | API-1 | +| `apps/runner/lib/oto/agent/registration_client.dart` | API-1 | +| `apps/runner/lib/oto/agent/oto_server_job_client.dart` | API-1 | +| `apps/runner/test/oto_agent_registration_test.dart` | API-1 | +| `apps/runner/test/oto_server_connection_smoke_test.dart` | API-1, API-3 | +| `services/core/internal/httpserver/routes.go` | API-2 | +| `services/core/internal/httpserver/server_test.go` | API-2 | +| `agent-test/local/agent-smoke.md` | API-3 | +| `agent-ops/rules/project/domain/agent/rules.md` | API-3 | + +## 최종 검증 + +```bash +cd apps/runner && dart test test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +cd services/core && go test -count=1 ./... +cd apps/runner && dart analyze +rg --sort path -n "OtoServerRegistrationClient|claimNextJob|jobs/claim|HTTP polling|compatibility|fallback|socket" apps/runner/lib apps/runner/test services/core/internal/httpserver agent-ops/rules/project/domain/agent/rules.md agent-test/local/agent-smoke.md +``` + +기대 결과: tests/analyzer 통과, grep 결과에서 HTTP path는 compatibility/fallback 맥락으로 구분된다. 모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/CODE_REVIEW-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/CODE_REVIEW-cloud-G07.md new file mode 100644 index 0000000..6fa74d4 --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/CODE_REVIEW-cloud-G07.md @@ -0,0 +1,152 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service blockers, generic scope changes, repeated failures, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only SDD/Milestone lock decisions in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-20 +task=m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `socket-smoke`: local/remote smoke 기준을 socket-first runner transport evidence로 갱신한다. +- Completion mode: check-on-pass +- Split dependency: + - `02+01_dispatch_state` PASS complete evidence가 있어야 구현을 시작한다. + - `03+01,02_runner_actions` PASS complete evidence가 있어야 구현을 시작한다. + - `04+01_compat_boundary` PASS complete evidence가 있어야 구현을 시작한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S06`: task=`socket-smoke`; evidence=`agent local smoke와 runner/core smoke가 socket-first lifecycle, dispatch, action, compatibility boundary를 검증한다.` +- Completion mode: spec-check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G07.md` -> `code_review_cloud_G07_N.log`, `PLAN-cloud-G07.md` -> `plan_cloud_G07_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/`로 이동한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] smoke 문서와 Makefile/test grouping 갱신 | [ ] | +| [API-2] runner socket smoke 대표성 보강 | [ ] | +| [API-3] full local smoke 기록 | [ ] | + +## 구현 체크리스트 + +- [ ] 선행 `02+01_dispatch_state`, `03+01,02_runner_actions`, `04+01_compat_boundary` PASS complete evidence를 확인한다. +- [ ] [API-1] `agent-test/local/agent-smoke.md`와 Makefile/test grouping을 socket-first 기준으로 갱신한다. 검증: `cd apps/runner && dart analyze` +- [ ] [API-2] runner socket smoke가 lifecycle/dispatch/action/compat boundary evidence를 대표하도록 보강한다. 검증: `cd apps/runner && dart test test/oto_server_connection_smoke_test.dart` +- [ ] [API-3] full local smoke 명령을 실행하고 결과를 기록한다. 검증: Go full test와 runner required test list +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G07_N.log`로 아카이브한다. +- [ ] active `PLAN-*-G??.md`를 `plan_cloud_G07_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/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/`를 `agent-task/archive/YYYY/MM/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-runner-proto-socket-transport-hardening/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active plan/review 파일 또는 follow-up plan을 작성하고 `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`를 남기지 않는다. + +## 계획 대비 변경 사항 + +_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ + +## 주요 설계 결정 + +_구현 에이전트가 주요 설계 결정 사항을 기록한다._ + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- smoke 문서가 실제 실행 가능한 명령만 요구하는지 확인한다. +- socket smoke가 HTTP registration/polling을 기본 evidence로 제출하지 않는지 확인한다. +- S06 evidence가 S02-S05 선행 결과를 무리하게 중복 구현하지 않고 종합 검증으로 닫는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### API-1 중간 검증 +```bash +$ cd apps/runner && dart analyze +(output) +``` + +### API-2 중간 검증 +```bash +$ cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +(output) +``` + +### API-3 중간 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +$ cd apps/runner && dart analyze +(output) +$ cd apps/runner && dart test test/oto_agent_migration_plan_test.dart test/oto_agent_bootstrap_script_test.dart test/oto_agent_cli_test.dart test/oto_agent_config_test.dart test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +(output) +``` + +### 최종 검증 +```bash +$ cd services/core && go test -count=1 ./... +(output) +$ cd apps/runner && dart analyze +(output) +$ cd apps/runner && dart test test/oto_agent_migration_plan_test.dart test/oto_agent_bootstrap_script_test.dart test/oto_agent_cli_test.dart test/oto_agent_config_test.dart test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +(output) +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. diff --git a/agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/PLAN-cloud-G07.md b/agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/PLAN-cloud-G07.md new file mode 100644 index 0000000..70e47ad --- /dev/null +++ b/agent-task/m-runner-proto-socket-transport-hardening/05+02,03,04_socket_smoke/PLAN-cloud-G07.md @@ -0,0 +1,168 @@ + + +# Plan - API + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-cloud-G07.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것까지가 구현입니다. 검증을 실행하고 실제 stdout/stderr를 붙인 뒤 active 파일을 그대로 두고 review ready로 보고하세요. 선택된 SDD 결정 또는 Milestone `구현 잠금 > 결정 필요` 항목이 막으면 review stub의 `사용자 리뷰 요청` 섹션을 채우고 멈추며, 직접 사용자에게 질문하거나 `USER_REVIEW.md`, `complete.log`, archive를 만들지 않습니다. 환경/secret/서비스 문제나 증거 공백은 일반 follow-up 대상으로 기록합니다. + +## 배경 + +Milestone의 마지막 socket-transport task는 local/remote smoke evidence를 socket-centered 기준으로 고정하는 것이다. 앞선 lifecycle, dispatch/state, runner-actions, compatibility-boundary 결과를 받아 `agent-test/local/agent-smoke.md`, Makefile/test grouping, runner smoke가 HTTP registration/polling을 기본 evidence처럼 쓰지 않도록 정리한다. + +## 사용자 리뷰 요청 흐름 + +사용자 리뷰 요청은 선택된 SDD 결정 또는 선택된 Milestone lock 결정이 실구현을 차단할 때만 active `CODE_REVIEW-cloud-G07.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 구현 중 직접 사용자 프롬프트, 채팅 선택지, `request_user_input`, `USER_REVIEW.md` 생성은 금지이며, code-review가 검증과 파일 생성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- Task ids: + - `socket-smoke`: local/remote smoke 기준을 socket-first runner transport evidence로 갱신한다. +- Completion mode: check-on-pass +- Split dependency: + - `02+01_dispatch_state` PASS complete evidence가 있어야 구현을 시작한다. + - `03+01,02_runner_actions` PASS complete evidence가 있어야 구현을 시작한다. + - `04+01_compat_boundary` PASS complete evidence가 있어야 구현을 시작한다. + +## Spec Targets + +- SDD: `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- Acceptance scenarios: + - `S06`: task=`socket-smoke`; evidence=`agent local smoke와 runner/core smoke가 socket-first lifecycle, dispatch, action, compatibility boundary를 검증한다.` +- Completion mode: spec-check-on-pass + +## 분석 결과 + +### 읽은 파일 + +- `agent-test/local/rules.md` +- `agent-test/local/agent-smoke.md` +- `agent-test/local/framework-smoke.md` +- `agent-ops/rules/project/domain/agent/rules.md` +- `agent-ops/rules/project/domain/framework/rules.md` +- `agent-roadmap/phase/control-plane-product-surface/milestones/runner-proto-socket-transport-hardening.md` +- `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` +- `Makefile` +- `apps/runner/test/oto_agent_registration_test.dart` +- `apps/runner/test/oto_server_connection_smoke_test.dart` +- `apps/runner/lib/oto/agent/agent_runner.dart` +- `apps/runner/lib/oto/agent/registration_client.dart` +- `services/core/internal/runnersocket/server.go` +- `services/core/internal/httpserver/server_test.go` + +### 테스트 환경 규칙 + +`test_env=local`을 적용했다. `agent-test/local/agent-smoke.md`의 agent 필수 명령, Makefile의 runner test list, Go core full test를 모두 socket-centered evidence로 맞춘다. 이 작업은 앞선 split 결과를 종합하므로 targeted tests만 통과해도 안 되고 smoke 문서 기준과 실제 명령이 함께 맞아야 한다. + +### 테스트 커버리지 공백 + +- `oto_server_connection_smoke_test.dart`의 첫 smoke는 HTTP registration client를 쓰고 있어 socket-first smoke의 대표 evidence로 부적합하다. +- `_startCore`는 socket listener env를 준비하지만 pass criteria가 socket lifecycle/dispatch/action을 명확히 요구하지 않는다. +- `agent-test/local/agent-smoke.md`는 runner transport hardening 전용 pass/fail criteria가 부족하다. + +### 심볼 참조 + +`oto_server_connection_smoke_test.dart`, `_startCore`, `OtoServerSocketRegistrationClient`, `OtoServerPushJobSession`, `Makefile`의 `test-runner`, `agent-test/local/agent-smoke.md`의 runner 명령을 중심으로 확인했다. + +### 분할 판단 + +이 작업은 최종 smoke 정리이므로 `dispatch-state`, `runner-actions`, `compat-boundary` 결과를 모두 기다린다. 선행 task가 완료되지 않았으면 smoke 기준을 고정하지 않는다. + +### 범위 결정 근거 + +이 plan은 smoke 문서/테스트 기준을 정리하고 필요한 작은 test grouping을 보강한다. lifecycle/state/action 자체 구현은 선행 split task에서 끝난 상태를 전제로 한다. + +### 빌드 등급 + +`cloud-G07`: smoke 기준이 이후 agent 작업의 완료 판정을 좌우한다. 문서와 실제 명령이 어긋나면 Milestone 전체가 잘못 완료될 수 있어 전체 local smoke 명령을 요구한다. + +## 구현 체크리스트 + +- [ ] 선행 `02+01_dispatch_state`, `03+01,02_runner_actions`, `04+01_compat_boundary` PASS complete evidence를 확인한다. +- [ ] [API-1] `agent-test/local/agent-smoke.md`와 Makefile/test grouping을 socket-first 기준으로 갱신한다. 검증: `cd apps/runner && dart analyze` +- [ ] [API-2] runner socket smoke가 lifecycle/dispatch/action/compat boundary evidence를 대표하도록 보강한다. 검증: `cd apps/runner && dart test test/oto_server_connection_smoke_test.dart` +- [ ] [API-3] full local smoke 명령을 실행하고 결과를 기록한다. 검증: Go full test와 runner required test list +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [API-1] smoke 문서와 Makefile/test grouping 갱신 + +문제: [agent-smoke.md](/config/workspace/oto/agent-test/local/agent-smoke.md:50)의 agent smoke 기준과 [Makefile](/config/workspace/oto/Makefile:7)의 runner test list가 socket-transport hardening의 pass criteria를 충분히 드러내지 않는다. + +해결 방법: + +`agent-test/local/agent-smoke.md`에 runner transport hardening 기준을 추가하거나 기존 agent smoke 명령 설명을 조정한다. Makefile test list가 이미 필요한 runner tests를 포함하면 변경하지 않고 evidence만 명시한다. 문서 변경은 사람이 실행할 수 있는 실제 명령과 일치해야 한다. + +수정 파일 및 체크리스트: + +- [ ] agent smoke 문서에 socket-first lifecycle/dispatch/action/compatibility evidence 기준 추가 +- [ ] Makefile runner test list가 smoke 문서와 어긋나지 않는지 확인 +- [ ] 불필요한 test command duplication은 피함 + +중간 검증: + +```bash +cd apps/runner && dart analyze +``` + +기대 결과: analyzer issue 없음. + +### [API-2] runner socket smoke 대표성 보강 + +문제: [oto_server_connection_smoke_test.dart](/config/workspace/oto/apps/runner/test/oto_server_connection_smoke_test.dart:1)는 server connection smoke를 포함하지만 HTTP registration/polling evidence와 socket evidence가 섞여 있다. Milestone 완료 evidence로는 socket-first scenario가 명확해야 한다. + +해결 방법: + +socket registration, server-assigned runner id, pushed `RunRequest`, report result, cancel/action or explicit excluded compatibility evidence가 한눈에 보이도록 smoke test names/groups를 정리한다. `_startCore`의 socket listener env를 계속 사용한다. + +수정 파일 및 체크리스트: + +- [ ] socket smoke group을 대표 evidence로 배치 +- [ ] HTTP compatibility smoke는 fallback group으로 분리 +- [ ] lifecycle/dispatch/action 선행 tests를 중복하지 않고 smoke-level assertion만 유지 + +중간 검증: + +```bash +cd apps/runner && dart test test/oto_server_connection_smoke_test.dart +``` + +기대 결과: smoke test 통과. + +### [API-3] full local smoke 기록 + +문제: Milestone 완료는 단일 Dart smoke만으로 충분하지 않다. core socket server/state tests와 runner required tests가 함께 통과해야 한다. + +해결 방법: + +최종 검증에서 core Go 전체, Dart analyzer, agent-smoke에 명시된 runner tests를 실행하고 CODE_REVIEW에 stdout/stderr를 붙인다. + +중간 검증: + +```bash +cd services/core && go test -count=1 ./... +cd apps/runner && dart analyze +cd apps/runner && dart test test/oto_agent_migration_plan_test.dart test/oto_agent_bootstrap_script_test.dart test/oto_agent_cli_test.dart test/oto_agent_config_test.dart test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +``` + +기대 결과: 모든 명령 통과. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `agent-test/local/agent-smoke.md` | API-1 | +| `Makefile` | API-1 | +| `apps/runner/test/oto_server_connection_smoke_test.dart` | API-2 | +| `apps/runner/test/oto_agent_registration_test.dart` | API-2 | +| `agent-roadmap/sdd/control-plane-product-surface/runner-proto-socket-transport-hardening/SDD.md` | API-1, API-3 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./... +cd apps/runner && dart analyze +cd apps/runner && dart test test/oto_agent_migration_plan_test.dart test/oto_agent_bootstrap_script_test.dart test/oto_agent_cli_test.dart test/oto_agent_config_test.dart test/oto_agent_registration_test.dart test/oto_server_connection_smoke_test.dart +``` + +기대 결과: core Go full test 통과, Dart analyzer 통과, agent required tests 통과. 모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/apps/runner/assets/script/powershell/oto_agent_bootstrap.ps1 b/apps/runner/assets/script/powershell/oto_agent_bootstrap.ps1 index 939ad3b..e6a691f 100644 --- a/apps/runner/assets/script/powershell/oto_agent_bootstrap.ps1 +++ b/apps/runner/assets/script/powershell/oto_agent_bootstrap.ps1 @@ -3,6 +3,7 @@ Set-StrictMode -Version Latest $ErrorActionPreference = 'Stop' $serverUrl = '' +$socketUrl = '' $agentId = '' $enrollmentToken = '' $releaseBaseUrl = '' @@ -18,6 +19,7 @@ while ($i -lt $args.Count) { switch ($args[$i]) { '--' { $i++ } '--server-url' { $serverUrl = $args[$i + 1]; $i += 2 } + '--socket-url' { $socketUrl = $args[$i + 1]; $i += 2 } '--agent-id' { $agentId = $args[$i + 1]; $i += 2 } '--enrollment-token' { $enrollmentToken = $args[$i + 1]; $i += 2 } '--release-base-url' { $releaseBaseUrl = $args[$i + 1]; $i += 2 } @@ -82,9 +84,14 @@ try { $agentAliasYaml = $agentAlias.Replace("'", "''") $enrollmentTokenYaml = $enrollmentToken.Replace("'", "''") $serverUrlYaml = $serverUrl.Replace("'", "''") + $socketUrlYaml = $socketUrl.Replace("'", "''") $installDirYaml = ($installDir -replace '\\', '/').Replace("'", "''") $workspaceRootYaml = ($workspaceRoot -replace '\\', '/').Replace("'", "''") $logDirYaml = ($logDir -replace '\\', '/').Replace("'", "''") + $socketUrlLine = '' + if ($socketUrl) { + $socketUrlLine = " socket_url: '$socketUrlYaml'`n" + } $configContent = @" agent: @@ -93,6 +100,7 @@ agent: enrollment_token: '$enrollmentTokenYaml' server: url: '$serverUrlYaml' +$socketUrlLine runtime: install_dir: '$installDirYaml' workspace_root: '$workspaceRootYaml' diff --git a/apps/runner/assets/script/shell/oto_agent_bootstrap.sh b/apps/runner/assets/script/shell/oto_agent_bootstrap.sh index 1d8468f..57af11a 100644 --- a/apps/runner/assets/script/shell/oto_agent_bootstrap.sh +++ b/apps/runner/assets/script/shell/oto_agent_bootstrap.sh @@ -2,6 +2,7 @@ set -euo pipefail server_url="" +socket_url="" agent_id="" enrollment_token="" release_base_url="" @@ -13,8 +14,9 @@ log_dir="${HOME}/.oto/agent/log" background="true" usage() { - echo "Usage: $0 --server-url --agent-id --enrollment-token --release-base-url [options]" + echo "Usage: $0 --server-url --socket-url --agent-id --enrollment-token --release-base-url [options]" echo "Options:" + echo " --socket-url Runner proto-socket URL (default: derived by agent)" echo " --agent-alias Alias for the agent" echo " --install-dir Directory to install OTO agent (default: \$HOME/.oto/bin)" echo " --config-path Path to config file (default: \$HOME/.oto/agent/config.yaml)" @@ -39,6 +41,10 @@ while [[ $# -gt 0 ]]; do server_url="$2" shift 2 ;; + --socket-url) + socket_url="$2" + shift 2 + ;; --agent-id) agent_id="$2" shift 2 @@ -155,6 +161,11 @@ agent: enrollment_token: "$(yaml_escape "$enrollment_token")" server: url: "$(yaml_escape "$server_url")" +EOF +if [[ -n "$socket_url" ]]; then + printf ' socket_url: "%s"\n' "$(yaml_escape "$socket_url")" >> "$config_path" +fi +cat <> "$config_path" runtime: install_dir: "$(yaml_escape "$install_dir")" workspace_root: "$(yaml_escape "$workspace_root")" diff --git a/apps/runner/lib/oto/agent/agent_config.dart b/apps/runner/lib/oto/agent/agent_config.dart index ba8ec65..9929de0 100644 --- a/apps/runner/lib/oto/agent/agent_config.dart +++ b/apps/runner/lib/oto/agent/agent_config.dart @@ -23,12 +23,13 @@ class AgentIdentityConfig { class ServerConnectionConfig { final String url; + final String? socketUrl; - const ServerConnectionConfig({required this.url}); + const ServerConnectionConfig({required this.url, this.socketUrl}); } class EdgeConnectionConfig extends ServerConnectionConfig { - const EdgeConnectionConfig({required super.url}); + const EdgeConnectionConfig({required super.url, super.socketUrl}); } class AgentRuntimeConfig { @@ -55,7 +56,8 @@ class AgentConfig { required this.runtime, }) : server = server ?? edge!; - EdgeConnectionConfig get edge => EdgeConnectionConfig(url: server.url); + EdgeConnectionConfig get edge => + EdgeConnectionConfig(url: server.url, socketUrl: server.socketUrl); static String _requiredString(Map map, String sectionName, String key) { if (!map.containsKey(key)) { @@ -146,6 +148,11 @@ class AgentConfig { } final serverUrl = _requiredString(connectionMap, connectionSection, 'url'); + final socketUrl = _optionalString( + connectionMap, + connectionSection, + 'socket_url', + ); final runtimeMap = root['runtime']; if (runtimeMap is! Map) { @@ -166,7 +173,7 @@ class AgentConfig { alias: alias, enrollmentToken: enrollmentToken, ), - server: ServerConnectionConfig(url: serverUrl), + server: ServerConnectionConfig(url: serverUrl, socketUrl: socketUrl), runtime: AgentRuntimeConfig( installDir: installDir, workspaceRoot: workspaceRoot, diff --git a/apps/runner/lib/oto/agent/agent_runner.dart b/apps/runner/lib/oto/agent/agent_runner.dart index d5cd59c..33b60fc 100644 --- a/apps/runner/lib/oto/agent/agent_runner.dart +++ b/apps/runner/lib/oto/agent/agent_runner.dart @@ -32,7 +32,7 @@ class DefaultAgentRunner implements AgentRunner { Future? shutdownFuture, }) : _client = client ?? - OtoServerRegistrationClient( + OtoServerSocketRegistrationClient( capabilityProvider: const CommandCatalogRunnerCapabilityProvider(), ), _onLog = onLog, @@ -71,20 +71,67 @@ class DefaultAgentRunner implements AgentRunner { // _awaitShutdown so signal listeners are not duplicated. final shutdownFuture = _shutdownFuture ?? _buildSignalShutdownFuture(); + var loopWaitedForShutdown = false; if (_runJobs != null) { await _runJobs(config, session); + } else if (session is OtoServerPushJobSession) { + await _pushJobLoop(config, session, shutdownFuture); + loopWaitedForShutdown = true; } else if (session is OtoServerJobSession) { // Default: run remote job loop when no custom job loop is injected. await _defaultJobLoop(config, session, shutdownFuture); + loopWaitedForShutdown = true; } - _log('Agent connected. Waiting for shutdown signal...'); - await shutdownFuture; + if (!loopWaitedForShutdown) { + _log('Agent connected. Waiting for shutdown signal...'); + await shutdownFuture; + } } finally { await session.close(); } } + /// Default proto-socket job loop: the server pushes RunRequest messages over + /// the open runner socket. Jobs are executed sequentially per agent process. + Future _pushJobLoop( + AgentConfig config, + OtoServerPushJobSession session, + Future shutdownFuture, + ) async { + final executor = RemoteRunExecutor( + parser: RunRequestParser(workspaceRoot: config.runtime.workspaceRoot), + jobClient: session.jobReporter, + onLog: _log, + ); + + var runChain = Future.value(); + final subscription = session.runRequests.listen((request) { + runChain = runChain.then((_) async { + if (request.jobId.isEmpty || request.executionId.isEmpty) { + _log('Received RunRequest without job_id or execution_id; skipping.'); + return; + } + final buildResult = await executor.runOnce( + jobId: request.jobId, + executionId: request.executionId, + runRequest: request, + ); + if (!buildResult.success) { + _log( + 'Job ${request.jobId} failed: ' + '${buildResult.message}', + ); + } + }); + }); + + _log('Agent connected via proto-socket. Waiting for pushed jobs...'); + await shutdownFuture; + await subscription.cancel(); + await runChain; + } + /// Default remote job loop: claim -> execute -> report, polling the /// OTO Server for new jobs. Uses [runOnce] so tests can drive a single /// iteration without an infinite loop. diff --git a/apps/runner/lib/oto/agent/oto_server_job_client.dart b/apps/runner/lib/oto/agent/oto_server_job_client.dart index e397dbb..40efc8d 100644 --- a/apps/runner/lib/oto/agent/oto_server_job_client.dart +++ b/apps/runner/lib/oto/agent/oto_server_job_client.dart @@ -5,6 +5,7 @@ import 'package:http/http.dart' as http; import 'package:oto/oto/agent/oto/runner.pb.dart' as oto; import 'package:oto/oto/core/build_result.dart'; import 'package:oto/oto/core/execution_context.dart'; +import 'package:proto_socket/proto_socket.dart'; class JobClaimResult { final bool accepted; @@ -123,8 +124,13 @@ class RunnerStatusResult { accepted: json['accepted'] == true, runnerId: (json['runner_id'] ?? json['runnerId'] ?? '').toString(), status: (json['status'] ?? '').toString(), - currentJobId: _emptyToNull((json['current_job_id'] ?? json['currentJobId'] ?? '').toString()), - currentExecutionId: _emptyToNull((json['current_execution_id'] ?? json['currentExecutionId'] ?? '').toString()), + currentJobId: _emptyToNull( + (json['current_job_id'] ?? json['currentJobId'] ?? '').toString(), + ), + currentExecutionId: _emptyToNull( + (json['current_execution_id'] ?? json['currentExecutionId'] ?? '') + .toString(), + ), message: _emptyToNull((json['message'] ?? '').toString()), ); } @@ -150,13 +156,36 @@ class RunnerSelfUpdateResult { success: json['success'] == true, accepted: json['accepted'] == true, deferred: json['deferred'] == true, - targetVersion: _emptyToNull((json['target_version'] ?? json['targetVersion'] ?? '').toString()), + targetVersion: _emptyToNull( + (json['target_version'] ?? json['targetVersion'] ?? '').toString(), + ), message: _emptyToNull((json['message'] ?? '').toString()), ); } } -class OtoServerJobClient { +abstract class RemoteJobReporter { + Future reportExecution({ + required String jobId, + required String executionId, + required BuildResult result, + }); + + Future appendLog({required String executionId, required String line}); + + Future reportArtifact({ + required String executionId, + required String name, + required String path, + }); + + Future reportArtifacts({ + required String executionId, + required List artifacts, + }); +} + +class OtoServerJobClient implements RemoteJobReporter { final String serverUrl; final String runnerId; final http.Client _client; @@ -198,6 +227,7 @@ class OtoServerJobClient { return claimJob(jobId: '', executionId: ''); } + @override Future reportExecution({ required String jobId, required String executionId, @@ -218,6 +248,7 @@ class OtoServerJobClient { return ExecutionReportResult.fromJson(response); } + @override Future appendLog({ required String executionId, required String line, @@ -244,6 +275,7 @@ class OtoServerJobClient { } } + @override Future reportArtifact({ required String executionId, required String name, @@ -276,6 +308,7 @@ class OtoServerJobClient { /// /// Stops and rethrows on the first failure so the caller can surface the HTTP /// error; partial reporting is preferred over silently swallowing failures. + @override Future reportArtifacts({ required String executionId, required List artifacts, @@ -306,9 +339,7 @@ class OtoServerJobClient { } Future fetchRunnerStatus() async { - final response = await _getJson( - _runnerUri('/status'), - ); + final response = await _getJson(_runnerUri('/status')); return RunnerStatusResult.fromJson(response); } @@ -321,10 +352,7 @@ class OtoServerJobClient { 'version': version, 'download_url': downloadUrl, }; - final response = await _postJson( - _runnerUri('/self-update'), - body, - ); + final response = await _postJson(_runnerUri('/self-update'), body); return RunnerSelfUpdateResult.fromJson(response); } @@ -369,10 +397,7 @@ class OtoServerJobClient { int expectedStatus = HttpStatus.ok, }) async { final response = await _client - .get( - uri, - headers: {'accept': 'application/json'}, - ) + .get(uri, headers: {'accept': 'application/json'}) .timeout(const Duration(seconds: 5)); if (response.statusCode != expectedStatus) { throw HttpException( @@ -387,6 +412,101 @@ class OtoServerJobClient { } } +class OtoServerSocketJobClient implements RemoteJobReporter { + final String runnerId; + final ProtobufClient _socket; + + OtoServerSocketJobClient({ + required this.runnerId, + required ProtobufClient socket, + }) : _socket = socket; + + @override + Future reportExecution({ + required String jobId, + required String executionId, + required BuildResult result, + }) async { + final request = oto.ExecutionReportRequest() + ..runnerId = runnerId + ..jobId = jobId + ..executionId = executionId + ..success = result.success + ..exitCode = result.exitCode + ..message = result.message + ..stepEvents.addAll(result.stepEvents.map(_stepEventReport)); + final response = await _socket + .sendRequest( + request, + ); + return ExecutionReportResult( + accepted: response.accepted, + errorMessage: _emptyToNull(response.errorMessage), + jobId: response.jobId, + executionId: response.executionId, + state: response.state, + ); + } + + @override + Future appendLog({ + required String executionId, + required String line, + }) async { + final request = oto.LogAppendRequest() + ..runnerId = runnerId + ..executionId = executionId + ..line = line; + final response = await _socket + .sendRequest(request); + if (!response.accepted) { + throw HttpException( + response.errorMessage.isNotEmpty + ? response.errorMessage + : 'log rejected', + ); + } + } + + @override + Future reportArtifact({ + required String executionId, + required String name, + required String path, + }) async { + final request = oto.ArtifactReportRequest() + ..runnerId = runnerId + ..executionId = executionId + ..name = name + ..path = path; + final response = await _socket + .sendRequest( + request, + ); + if (!response.accepted) { + throw HttpException( + response.errorMessage.isNotEmpty + ? response.errorMessage + : 'artifact rejected', + ); + } + } + + @override + Future reportArtifacts({ + required String executionId, + required List artifacts, + }) async { + for (final artifact in artifacts) { + await reportArtifact( + executionId: executionId, + name: artifact.name, + path: artifact.path, + ); + } + } +} + Uri _apiUri(String serverUrl, String endpointPath) { var raw = serverUrl; if (!raw.contains('://')) { diff --git a/apps/runner/lib/oto/agent/registration_client.dart b/apps/runner/lib/oto/agent/registration_client.dart index 4da1fae..bd4c42d 100644 --- a/apps/runner/lib/oto/agent/registration_client.dart +++ b/apps/runner/lib/oto/agent/registration_client.dart @@ -7,6 +7,8 @@ import 'package:oto/oto/agent/agent_config.dart'; import 'package:oto/oto/agent/oto_server_job_client.dart'; import 'package:oto/oto/agent/oto/runner.pb.dart' as oto; import 'package:oto/oto/agent/runner_capability_provider.dart'; +import 'package:protobuf/protobuf.dart'; +import 'package:proto_socket/proto_socket.dart'; const otoRunnerProtocolVersion = 'oto.runner.v1'; const otoRunnerCapabilityName = 'oto-runner'; @@ -98,6 +100,11 @@ abstract class OtoServerJobSession implements AgentSession { OtoServerJobClient get jobs; } +abstract class OtoServerPushJobSession implements AgentSession { + RemoteJobReporter get jobReporter; + Stream get runRequests; +} + abstract class RegistrationClient { Future openSession(AgentConfig config); @@ -111,6 +118,62 @@ abstract class RegistrationClient { } } +class OtoServerSocketRegistrationClient extends RegistrationClient { + final RunnerCapabilityProvider _capabilityProvider; + final Duration _heartbeatInterval; + + OtoServerSocketRegistrationClient({ + Iterable? commandTypes, + RunnerCapabilityProvider? capabilityProvider, + Duration heartbeatInterval = const Duration(seconds: 30), + }) : _capabilityProvider = + capabilityProvider ?? + _StaticRunnerCapabilityProvider(commandTypes ?? const []), + _heartbeatInterval = heartbeatInterval; + + @override + Future openSession(AgentConfig config) async { + final endpoint = _RunnerSocketEndpoint.fromConfig(config.server); + final socket = await ProtobufClient.connect( + endpoint.host, + endpoint.port, + ).timeout(const Duration(seconds: 5)); + final client = _OtoRunnerSocketClient(socket); + try { + final response = await client + .sendRequest( + buildRegisterRunnerRequest(config), + timeout: const Duration(seconds: 5), + ); + final result = RegistrationResult.fromOtoResponse(response); + final runnerId = result.runnerId ?? config.agent.id; + return _OtoServerSocketAgentSession( + result, + client, + runnerId, + heartbeatInterval: _heartbeatInterval, + ); + } catch (_) { + await client.close(); + rethrow; + } + } + + oto.RegisterRunnerRequest buildRegisterRunnerRequest(AgentConfig config) { + final capability = _capabilityProvider.snapshot(); + return oto.RegisterRunnerRequest() + ..enrollmentToken = config.agent.enrollmentToken + ..runnerId = config.agent.id + ..alias = config.agent.alias ?? '' + ..protocolVersion = otoRunnerProtocolVersion + ..capability = (oto.RunnerCapability() + ..name = capability.name + ..version = capability.version) + ..commandCatalog = (oto.CommandCatalogSummary() + ..commandTypes.addAll(capability.commandTypes)); + } +} + class OtoServerRegistrationClient extends RegistrationClient { final http.Client? _client; final RunnerCapabilityProvider _capabilityProvider; @@ -150,12 +213,13 @@ class OtoServerRegistrationClient extends RegistrationClient { throw const FormatException('Invalid registration response body'); } final result = RegistrationResult.fromOtoJson(decoded); + final runnerId = result.runnerId ?? config.agent.id; return _OtoServerAgentSession( result, client, shouldCloseClient, config.server.url, - config.agent.id, + runnerId, heartbeatInterval: _heartbeatInterval, ); } catch (_) { @@ -245,6 +309,133 @@ class OtoServerRegistrationClient extends RegistrationClient { }; } +class _OtoRunnerSocketClient extends ProtobufClient { + _OtoRunnerSocketClient(Socket socket) + : super(socket, 30, 45, _otoRunnerSocketParserMap()); +} + +class _OtoServerSocketAgentSession implements OtoServerPushJobSession { + final _OtoRunnerSocketClient _client; + final String _runnerId; + final StreamController _runRequests = + StreamController(); + Timer? _heartbeatTimer; + bool _closed = false; + + @override + final RegistrationResult result; + + @override + late final RemoteJobReporter jobReporter; + + @override + Stream get runRequests => _runRequests.stream; + + _OtoServerSocketAgentSession( + this.result, + this._client, + this._runnerId, { + required Duration heartbeatInterval, + }) { + jobReporter = OtoServerSocketJobClient( + runnerId: _runnerId, + socket: _client, + ); + _client.addListener((request) { + if (!_runRequests.isClosed) { + _runRequests.add(request); + } + }); + if (result.accepted) { + _heartbeatTimer = Timer.periodic( + heartbeatInterval, + (_) => _sendHeartbeat(), + ); + scheduleMicrotask(() => _sendHeartbeat()); + } + } + + Future _sendHeartbeat() async { + if (_closed) return; + try { + await _client.sendRequest( + oto.HeartbeatRequest() + ..runnerId = _runnerId + ..status = oto.HeartbeatStatus.HEARTBEAT_STATUS_HEALTHY, + timeout: const Duration(seconds: 5), + ); + } catch (_) { + // Ignore background heartbeat errors; proto-socket heartbeat owns liveness. + } + } + + @override + Future close() async { + if (_closed) return; + _closed = true; + _heartbeatTimer?.cancel(); + _heartbeatTimer = null; + unawaited(_runRequests.close()); + await _client.close(); + } +} + +class _RunnerSocketEndpoint { + final String host; + final int port; + + const _RunnerSocketEndpoint({required this.host, required this.port}); + + factory _RunnerSocketEndpoint.fromConfig(ServerConnectionConfig config) { + final raw = config.socketUrl ?? _deriveSocketUrl(config.url); + var parsedRaw = raw; + if (!parsedRaw.contains('://')) { + parsedRaw = 'tcp://$parsedRaw'; + } + final uri = Uri.parse(parsedRaw); + if (uri.host.isEmpty) { + throw FormatException('Invalid runner socket url: "$raw"'); + } + return _RunnerSocketEndpoint( + host: uri.host, + port: uri.hasPort ? uri.port : 18080, + ); + } + + static String _deriveSocketUrl(String serverUrl) { + var raw = serverUrl; + if (!raw.contains('://')) { + raw = 'http://$raw'; + } + final uri = Uri.parse(raw); + if (uri.host.isEmpty) { + throw FormatException('Invalid server url: "$serverUrl"'); + } + return 'tcp://${uri.host}:18080'; + } +} + +Map)> _otoRunnerSocketParserMap() { + return { + oto.RegisterRunnerResponse.getDefault().info_.qualifiedMessageName: + oto.RegisterRunnerResponse.fromBuffer, + oto.HeartbeatResponse.getDefault().info_.qualifiedMessageName: + oto.HeartbeatResponse.fromBuffer, + oto.RunRequest.getDefault().info_.qualifiedMessageName: + oto.RunRequest.fromBuffer, + oto.ExecutionReportResponse.getDefault().info_.qualifiedMessageName: + oto.ExecutionReportResponse.fromBuffer, + oto.LogAppendResponse.getDefault().info_.qualifiedMessageName: + oto.LogAppendResponse.fromBuffer, + oto.ArtifactReportResponse.getDefault().info_.qualifiedMessageName: + oto.ArtifactReportResponse.fromBuffer, + oto.CancelRunRequest.getDefault().info_.qualifiedMessageName: + oto.CancelRunRequest.fromBuffer, + oto.SelfUpdateRequest.getDefault().info_.qualifiedMessageName: + oto.SelfUpdateRequest.fromBuffer, + }; +} + class _StaticRunnerCapabilityProvider implements RunnerCapabilityProvider { final List _commandTypes; diff --git a/apps/runner/lib/oto/agent/remote_run_executor.dart b/apps/runner/lib/oto/agent/remote_run_executor.dart index 91d5e7e..967a9e7 100644 --- a/apps/runner/lib/oto/agent/remote_run_executor.dart +++ b/apps/runner/lib/oto/agent/remote_run_executor.dart @@ -53,12 +53,16 @@ class RunRequestParser { } throw ArgumentError( - 'RunRequest has neither pipelineYaml nor pipelineYamlPath.'); + 'RunRequest has neither pipelineYaml nor pipelineYamlPath.', + ); } String _resolvePathConfinement(String rawPath) { - return resolveWithinWorkspace(rawPath, workspaceRoot, - label: 'pipelineYamlPath'); + return resolveWithinWorkspace( + rawPath, + workspaceRoot, + label: 'pipelineYamlPath', + ); } Map _toMap(Map fields) { @@ -74,8 +78,11 @@ class RunRequestParser { /// /// `path.equals`/`path.isWithin` correctly reject sibling-prefix attacks such as /// `/var/workspace2` against a root of `/var/workspace`. -String resolveWithinWorkspace(String rawPath, String workspaceRoot, - {required String label}) { +String resolveWithinWorkspace( + String rawPath, + String workspaceRoot, { + required String label, +}) { final canonicalRoot = path.canonicalize(workspaceRoot); final absolute = path.canonicalize( path.isAbsolute(rawPath) ? rawPath : path.join(canonicalRoot, rawPath), @@ -83,7 +90,8 @@ String resolveWithinWorkspace(String rawPath, String workspaceRoot, if (!path.equals(canonicalRoot, absolute) && !path.isWithin(canonicalRoot, absolute)) { throw ArgumentError( - '$label escapes workspace: "$rawPath" resolved to "$absolute"'); + '$label escapes workspace: "$rawPath" resolved to "$absolute"', + ); } return absolute; } @@ -95,26 +103,26 @@ String resolveWithinWorkspace(String rawPath, String workspaceRoot, /// infinite loop. class RemoteRunExecutor { final RunRequestParser _parser; - final OtoServerJobClient _jobClient; + final RemoteJobReporter _jobClient; final void Function(String)? _onLog; final Duration pollInterval; RemoteRunExecutor({ required RunRequestParser parser, - required OtoServerJobClient jobClient, + required RemoteJobReporter jobClient, void Function(String)? onLog, this.pollInterval = const Duration(seconds: 2), - }) : _parser = parser, - _jobClient = jobClient, - _onLog = onLog; + }) : _parser = parser, + _jobClient = jobClient, + _onLog = onLog; /// Executes a single job identified by [jobId] and [executionId]. /// /// Flow: /// 1. Parse the [RunRequest] – if this fails, a failure report is sent. - /// 2. Log "run started" to server via [OtoServerJobClient.appendLog]. + /// 2. Log "run started" to server via [RemoteJobReporter.appendLog]. /// 3. Run [Application.build] with the resolved YAML + merged variables. - /// 4. Always call [OtoServerJobClient.reportExecution] with the result + /// 4. Always call [RemoteJobReporter.reportExecution] with the result /// (success or failure), including failures from steps 1–3. /// /// Returns the [BuildResult] regardless of success or failure. @@ -221,7 +229,8 @@ class RemoteRunExecutor { if (declarations == null) return const []; if (declarations is! List) { throw ArgumentError( - 'property.artifacts must be a list of {name, path} maps.'); + 'property.artifacts must be a list of {name, path} maps.', + ); } final result = []; @@ -229,21 +238,27 @@ class RemoteRunExecutor { final entry = declarations[i]; if (entry is! Map) { throw ArgumentError( - 'property.artifacts[$i] must be a map with name and path.'); + 'property.artifacts[$i] must be a map with name and path.', + ); } final name = entry['name']; if (name is! String || name.trim().isEmpty) { throw ArgumentError( - 'property.artifacts[$i] "name" must be a non-empty string.'); + 'property.artifacts[$i] "name" must be a non-empty string.', + ); } final declaredPath = entry['path']; if (declaredPath is! String || declaredPath.trim().isEmpty) { throw ArgumentError( - 'property.artifacts[$i] (name: "$name") "path" must be a non-empty string.'); + 'property.artifacts[$i] (name: "$name") "path" must be a non-empty string.', + ); } // Confine the declared path; the declared form is kept for reporting. - resolveWithinWorkspace(declaredPath, workspaceRoot, - label: 'property.artifacts[$i] path'); + resolveWithinWorkspace( + declaredPath, + workspaceRoot, + label: 'property.artifacts[$i] path', + ); result.add(BuildArtifact(name: name, path: declaredPath)); } return result; @@ -304,4 +319,4 @@ class RemoteRunExecutor { print('[RemoteRunExecutor] $message'); } } -} \ No newline at end of file +} diff --git a/apps/runner/test/oto_server_connection_smoke_test.dart b/apps/runner/test/oto_server_connection_smoke_test.dart index 4e6786b..97d328e 100644 --- a/apps/runner/test/oto_server_connection_smoke_test.dart +++ b/apps/runner/test/oto_server_connection_smoke_test.dart @@ -24,12 +24,7 @@ void main() { final serverAddr = '$_host:$port'; // Start Go OTO Server in background - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -150,14 +145,9 @@ void main() { final serverAddr = '$_host:$port'; // Start Go OTO Server in background - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: { - 'OTO_CORE_ADDR': serverAddr, - 'OTO_RUNNER_RELEASE_BASE_URL': 'https://example.com/releases', - }, + final serverProcess = await _startCore( + serverAddr, + releaseBaseUrl: 'https://example.com/releases', ); final output = StringBuffer(); @@ -355,12 +345,7 @@ fi const jobId = 'oto-smoke-job'; const executionId = 'oto-smoke-execution'; - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -538,12 +523,7 @@ fi const jobId = 'remote-smoke-job'; const executionId = 'remote-smoke-exec'; - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -574,8 +554,11 @@ fi commandTypes: ['Shell', 'Git'], ); final session = await regClient.openSession(agentConfig); - expect(session.result.accepted, isTrue, - reason: 'Registration was rejected'); + expect( + session.result.accepted, + isTrue, + reason: 'Registration was rejected', + ); final jobs = (session as OtoServerJobSession).jobs; final httpClient = http.Client(); @@ -605,8 +588,11 @@ pipeline: }, }), ); - expect(createResp.statusCode, equals(201), - reason: 'Create job failed: ${createResp.body}'); + expect( + createResp.statusCode, + equals(201), + reason: 'Create job failed: ${createResp.body}', + ); final claim = await jobs.claimJob( jobId: jobId, @@ -629,8 +615,11 @@ pipeline: runRequest: claim.runRequest!, ); - expect(buildResult.success, isTrue, - reason: 'Execution failed: ${buildResult.message}'); + expect( + buildResult.success, + isTrue, + reason: 'Execution failed: ${buildResult.message}', + ); expect(buildResult.exitCode, 0); expect(buildResult.stepEvents, isNotEmpty); @@ -642,12 +631,10 @@ pipeline: final logsResp = await httpClient.get( Uri.parse('http://$serverAddr/api/v1/executions/$executionId/logs'), ); - final logsJson = - jsonDecode(logsResp.body) as Map; + final logsJson = jsonDecode(logsResp.body) as Map; final logsList = logsJson['logs'] as List; final lines = logsList - .map((e) => - (e as Map)['Line'] ?? e['line']) + .map((e) => (e as Map)['Line'] ?? e['line']) .map((l) => l.toString()) .toList(); expect(lines, contains('run started for job $jobId')); @@ -655,12 +642,10 @@ pipeline: // Verify execution state final execResp = await httpClient.get( - Uri.parse( - 'http://$serverAddr/api/v1/executions/$executionId'), + Uri.parse('http://$serverAddr/api/v1/executions/$executionId'), ); expect(execResp.statusCode, 200); - final execJson = - jsonDecode(execResp.body) as Map; + final execJson = jsonDecode(execResp.body) as Map; expect(execJson['state'], equals('succeeded')); expect(execJson['job_id'], equals(jobId)); @@ -692,12 +677,7 @@ pipeline: const jobId = 'var-merge-job'; const executionId = 'var-merge-exec'; - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -726,7 +706,11 @@ pipeline: final regClient = OtoServerRegistrationClient(commandTypes: ['Print']); final session = await regClient.openSession(agentConfig); - expect(session.result.accepted, isTrue, reason: 'Registration rejected'); + expect( + session.result.accepted, + isTrue, + reason: 'Registration rejected', + ); final jobs = (session as OtoServerJobSession).jobs; final httpClient = http.Client(); @@ -758,8 +742,11 @@ pipeline: }, }), ); - expect(createResp.statusCode, equals(201), - reason: 'Create job failed: ${createResp.body}'); + expect( + createResp.statusCode, + equals(201), + reason: 'Create job failed: ${createResp.body}', + ); final claim = await jobs.claimJob( jobId: jobId, @@ -782,10 +769,16 @@ pipeline: runRequest: claim.runRequest!, ); - expect(buildResult.success, isTrue, - reason: 'Execution failed: ${buildResult.message}'); - expect(buildResult.stepEvents, isNotEmpty, - reason: 'Expected step events from the build'); + expect( + buildResult.success, + isTrue, + reason: 'Execution failed: ${buildResult.message}', + ); + expect( + buildResult.stepEvents, + isNotEmpty, + reason: 'Expected step events from the build', + ); // REVIEW_API2-2: assert the remote variable actually overrode the // YAML value. After a successful build, Application.instance.property @@ -793,16 +786,24 @@ pipeline: // to resolve ``. It must be the remote 'release', // not the YAML default 'original'. final mergedFlavor = Application.instance.property['FLAVOR']; - expect(mergedFlavor, equals('release'), - reason: - 'remote variable FLAVOR was not substituted; got: $mergedFlavor'); - expect(mergedFlavor, isNot(equals('original')), - reason: - 'YAML default "original" leaked through instead of the remote override'); - - expect(capturedLogs.any((l) => l.contains('success=true')), isTrue, - reason: 'Expected successful execution report; logs: $capturedLogs'); + expect( + mergedFlavor, + equals('release'), + reason: + 'remote variable FLAVOR was not substituted; got: $mergedFlavor', + ); + expect( + mergedFlavor, + isNot(equals('original')), + reason: + 'YAML default "original" leaked through instead of the remote override', + ); + expect( + capturedLogs.any((l) => l.contains('success=true')), + isTrue, + reason: 'Expected successful execution report; logs: $capturedLogs', + ); } finally { httpClient.close(); await session.close(); @@ -831,12 +832,7 @@ pipeline: const jobId = 'bad-prop-job'; const executionId = 'bad-prop-exec'; - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -865,7 +861,11 @@ pipeline: final regClient = OtoServerRegistrationClient(commandTypes: ['Print']); final session = await regClient.openSession(agentConfig); - expect(session.result.accepted, isTrue, reason: 'Registration rejected'); + expect( + session.result.accepted, + isTrue, + reason: 'Registration rejected', + ); final jobs = (session as OtoServerJobSession).jobs; final httpClient = http.Client(); @@ -897,8 +897,11 @@ pipeline: }, }), ); - expect(createResp.statusCode, equals(201), - reason: 'Create job failed: ${createResp.body}'); + expect( + createResp.statusCode, + equals(201), + reason: 'Create job failed: ${createResp.body}', + ); final claim = await jobs.claimJob( jobId: jobId, @@ -922,8 +925,11 @@ pipeline: // Build must fail on the invalid property; the merge must not have // rewritten it into a valid map. - expect(buildResult.success, isFalse, - reason: 'Expected failure from invalid property type'); + expect( + buildResult.success, + isFalse, + reason: 'Expected failure from invalid property type', + ); // Server job should be in failed state because reportExecution ran. final jobResp = await httpClient.get( @@ -931,9 +937,12 @@ pipeline: ); expect(jobResp.statusCode, 200); final jobJson = jsonDecode(jobResp.body) as Map; - expect(jobJson['state'], equals('failed'), - reason: - 'Server did not receive failure report; got: ${jobJson['state']}'); + expect( + jobJson['state'], + equals('failed'), + reason: + 'Server did not receive failure report; got: ${jobJson['state']}', + ); } finally { httpClient.close(); await session.close(); @@ -962,12 +971,7 @@ pipeline: const jobId = 'parse-fail-job'; const executionId = 'parse-fail-exec'; - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -996,7 +1000,11 @@ pipeline: final regClient = OtoServerRegistrationClient(commandTypes: ['Shell']); final session = await regClient.openSession(agentConfig); - expect(session.result.accepted, isTrue, reason: 'Registration rejected'); + expect( + session.result.accepted, + isTrue, + reason: 'Registration rejected', + ); final jobs = (session as OtoServerJobSession).jobs; final httpClient = http.Client(); @@ -1015,8 +1023,11 @@ pipeline: }, }), ); - expect(createResp.statusCode, equals(201), - reason: 'Create job failed: ${createResp.body}'); + expect( + createResp.statusCode, + equals(201), + reason: 'Create job failed: ${createResp.body}', + ); final claim = await jobs.claimJob( jobId: jobId, @@ -1040,8 +1051,11 @@ pipeline: ); // Build should have failed. - expect(buildResult.success, isFalse, - reason: 'Expected failure from parse error'); + expect( + buildResult.success, + isFalse, + reason: 'Expected failure from parse error', + ); // Server job should be in failed state because reportExecution was called. final jobResp = await httpClient.get( @@ -1049,8 +1063,12 @@ pipeline: ); expect(jobResp.statusCode, 200); final jobJson = jsonDecode(jobResp.body) as Map; - expect(jobJson['state'], equals('failed'), - reason: 'Server did not receive failure report; got: ${jobJson['state']}'); + expect( + jobJson['state'], + equals('failed'), + reason: + 'Server did not receive failure report; got: ${jobJson['state']}', + ); } finally { httpClient.close(); await session.close(); @@ -1079,12 +1097,7 @@ pipeline: const jobId = 'artifact-smoke-job'; const executionId = 'artifact-smoke-exec'; - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -1113,7 +1126,11 @@ pipeline: final regClient = OtoServerRegistrationClient(commandTypes: ['Print']); final session = await regClient.openSession(agentConfig); - expect(session.result.accepted, isTrue, reason: 'Registration rejected'); + expect( + session.result.accepted, + isTrue, + reason: 'Registration rejected', + ); final jobs = (session as OtoServerJobSession).jobs; final httpClient = http.Client(); @@ -1147,8 +1164,11 @@ pipeline: }, }), ); - expect(createResp.statusCode, equals(201), - reason: 'Create job failed: ${createResp.body}'); + expect( + createResp.statusCode, + equals(201), + reason: 'Create job failed: ${createResp.body}', + ); final claim = await jobs.claimJob( jobId: jobId, @@ -1171,8 +1191,11 @@ pipeline: runRequest: claim.runRequest!, ); - expect(buildResult.success, isTrue, - reason: 'Execution failed: ${buildResult.message}'); + expect( + buildResult.success, + isTrue, + reason: 'Execution failed: ${buildResult.message}', + ); expect(buildResult.artifacts, hasLength(1)); expect(buildResult.artifacts.first.name, 'smoke-report'); @@ -1215,12 +1238,7 @@ pipeline: const jobId = 'smoke-job-cancel'; const executionId = 'smoke-exec-cancel'; - final serverProcess = await Process.start( - 'go', - ['run', './cmd/oto-core'], - workingDirectory: '../../services/core', - environment: {'OTO_CORE_ADDR': serverAddr}, - ); + final serverProcess = await _startCore(serverAddr); final output = StringBuffer(); final stdoutSub = serverProcess.stdout @@ -1333,6 +1351,223 @@ pipeline: }, timeout: const Timeout(Duration(seconds: 30)), ); + + test( + 'OtoServerSocketRegistrationClient socket session sends heartbeat and disconnects on close', + () async { + final port = await _freePort(); + final serverAddr = '$_host:$port'; + + final (:process, :socketAddr) = await _startCoreWithSocket(serverAddr); + + final output = StringBuffer(); + final stdoutSub = process.stdout + .transform(systemEncoding.decoder) + .listen(output.write, onError: output.write); + final stderrSub = process.stderr + .transform(systemEncoding.decoder) + .listen(output.write, onError: output.write); + + try { + await _waitForPort(_host, port, process, output); + + final agentConfig = AgentConfig( + agent: const AgentIdentityConfig( + id: _runnerId, + alias: _runnerAlias, + enrollmentToken: _token, + ), + server: ServerConnectionConfig( + url: 'http://$serverAddr', + socketUrl: 'tcp://$socketAddr', + ), + runtime: const AgentRuntimeConfig( + installDir: '/tmp/install', + workspaceRoot: '/tmp/workspace', + logDir: '/tmp/log', + ), + ); + + final client = OtoServerSocketRegistrationClient( + commandTypes: ['Shell', 'Git'], + heartbeatInterval: const Duration(milliseconds: 200), + ); + + final session = await client.openSession(agentConfig); + expect(session, isA()); + expect(session.result.accepted, isTrue); + expect(session.result.runnerId, _runnerId); + + // Capture runRequests stream completion after close(). + final streamDone = Completer(); + final sub = (session as OtoServerPushJobSession).runRequests.listen( + (_) {}, + onDone: streamDone.complete, + ); + + final httpClient = http.Client(); + try { + final isOnline = await _pollRunnerStatus( + httpClient, + serverAddr, + _runnerId, + 'online', + ); + expect( + isOnline, + isTrue, + reason: 'socket runner did not go online in registry', + ); + + // close() must cancel the heartbeat timer and close runRequests. + await session.close(); + await streamDone.future.timeout( + const Duration(seconds: 2), + onTimeout: () => fail('runRequests stream did not close after session.close()'), + ); + await sub.cancel(); + + final isDisconnected = await _pollRunnerStatus( + httpClient, + serverAddr, + _runnerId, + 'disconnected', + timeout: const Duration(seconds: 5), + ); + expect( + isDisconnected, + isTrue, + reason: 'socket runner did not transition to disconnected', + ); + } finally { + httpClient.close(); + } + } finally { + process.kill(ProcessSignal.sigterm); + await process.exitCode.timeout( + const Duration(seconds: 5), + onTimeout: () { + process.kill(ProcessSignal.sigkill); + return process.exitCode; + }, + ); + await stdoutSub.cancel(); + await stderrSub.cancel(); + } + }, + timeout: const Timeout(Duration(seconds: 30)), + ); + + test( + 'OtoServerSocketRegistrationClient socket session close is idempotent after server disconnect', + () async { + final port = await _freePort(); + final serverAddr = '$_host:$port'; + + final (:process, :socketAddr) = await _startCoreWithSocket(serverAddr); + + final output = StringBuffer(); + final stdoutSub = process.stdout + .transform(systemEncoding.decoder) + .listen(output.write, onError: output.write); + final stderrSub = process.stderr + .transform(systemEncoding.decoder) + .listen(output.write, onError: output.write); + + try { + await _waitForPort(_host, port, process, output); + + final agentConfig = AgentConfig( + agent: const AgentIdentityConfig( + id: _runnerId, + alias: _runnerAlias, + enrollmentToken: _token, + ), + server: ServerConnectionConfig( + url: 'http://$serverAddr', + socketUrl: 'tcp://$socketAddr', + ), + runtime: const AgentRuntimeConfig( + installDir: '/tmp/install', + workspaceRoot: '/tmp/workspace', + logDir: '/tmp/log', + ), + ); + + final client = OtoServerSocketRegistrationClient( + commandTypes: ['Shell'], + // Short interval so in-flight heartbeats exercise the catch block. + heartbeatInterval: const Duration(milliseconds: 50), + ); + + final session = await client.openSession(agentConfig); + expect(session.result.accepted, isTrue); + + // Graceful server shutdown: closes all proto-socket clients cleanly, + // sending EOF to the Dart side. In-flight heartbeat sendRequest calls + // receive StateError('connection closed'), which is swallowed by the + // _sendHeartbeat catch block. + process.kill(ProcessSignal.sigterm); + await process.exitCode.timeout(const Duration(seconds: 5)); + + // Allow the EOF to propagate and the transport to auto-close. + await Future.delayed(const Duration(milliseconds: 300)); + + // session.close() must complete without throwing even though the + // proto-socket transport is already closed by the EOF handler. + await expectLater(session.close(), completes); + } finally { + process.kill(ProcessSignal.sigkill); + await process.exitCode.timeout( + const Duration(seconds: 2), + onTimeout: () => -1, + ); + await stdoutSub.cancel(); + await stderrSub.cancel(); + } + }, + timeout: const Timeout(Duration(seconds: 30)), + ); +} + +// ─── Socket session lifecycle tests ────────────────────────────────────────── + +typedef _CoreWithSocket = ({Process process, String socketAddr}); + +Future<_CoreWithSocket> _startCoreWithSocket(String serverAddr) async { + final socketPort = await _freePort(); + final socketAddr = '$_host:$socketPort'; + final process = await Process.start( + 'go', + ['run', './cmd/oto-core'], + workingDirectory: '../../services/core', + environment: { + 'OTO_CORE_ADDR': serverAddr, + 'OTO_RUNNER_SOCKET_ADDR': socketAddr, + 'OTO_RUNNER_SOCKET_PUBLIC_URL': 'tcp://$socketAddr', + }, + ); + return (process: process, socketAddr: socketAddr); +} + +Future _pollRunnerStatus( + http.Client client, + String serverAddr, + String runnerId, + String expected, { + Duration timeout = const Duration(seconds: 10), +}) async { + final url = Uri.parse('http://$serverAddr/api/v1/runners/$runnerId'); + final deadline = DateTime.now().add(timeout); + while (DateTime.now().isBefore(deadline)) { + final resp = await client.get(url); + if (resp.statusCode == 200) { + final data = jsonDecode(resp.body); + if (data['status'] == expected) return true; + } + await Future.delayed(const Duration(milliseconds: 100)); + } + return false; } Future _freePort() async { @@ -1342,6 +1577,24 @@ Future _freePort() async { return port; } +Future _startCore(String serverAddr, {String? releaseBaseUrl}) async { + final socketPort = await _freePort(); + final environment = { + 'OTO_CORE_ADDR': serverAddr, + 'OTO_RUNNER_SOCKET_ADDR': '$_host:$socketPort', + 'OTO_RUNNER_SOCKET_PUBLIC_URL': 'tcp://$_host:$socketPort', + }; + if (releaseBaseUrl != null) { + environment['OTO_RUNNER_RELEASE_BASE_URL'] = releaseBaseUrl; + } + return Process.start( + 'go', + ['run', './cmd/oto-core'], + workingDirectory: '../../services/core', + environment: environment, + ); +} + Future _waitForPort( String host, int port, diff --git a/services/core/cmd/oto-core/main.go b/services/core/cmd/oto-core/main.go index eba75f8..3837ab3 100644 --- a/services/core/cmd/oto-core/main.go +++ b/services/core/cmd/oto-core/main.go @@ -19,6 +19,12 @@ func main() { log.Printf("Starting OTO Core server on %s", addr) server := httpserver.NewServer(addr) + runnerSocketAddr := os.Getenv("OTO_RUNNER_SOCKET_ADDR") + if runnerSocketAddr == "" { + runnerSocketAddr = "127.0.0.1:18080" + } + server.EnableRunnerSocket(runnerSocketAddr) + log.Printf("Starting OTO runner proto-socket listener on %s", runnerSocketAddr) // Channel to listen for errors from the server run serverError := make(chan error, 1) diff --git a/services/core/go.mod b/services/core/go.mod index af5f597..6b4e3de 100644 --- a/services/core/go.mod +++ b/services/core/go.mod @@ -2,4 +2,11 @@ module github.com/toki/oto/services/core go 1.26 -require google.golang.org/protobuf v1.36.11 +require ( + git.toki-labs.com/toki/proto-socket/go v0.0.0 + google.golang.org/protobuf v1.36.11 +) + +require nhooyr.io/websocket v1.8.17 // indirect + +replace git.toki-labs.com/toki/proto-socket/go => ../../../proto-socket/go diff --git a/services/core/go.sum b/services/core/go.sum index 296be18..f29b6d5 100644 --- a/services/core/go.sum +++ b/services/core/go.sum @@ -2,3 +2,5 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= +nhooyr.io/websocket v1.8.17 h1:KEVeLJkUywCKVsnLIDlD/5gtayKp8VoCkksHCGGfT9Y= +nhooyr.io/websocket v1.8.17/go.mod h1:rN9OFWIUwuxg4fR5tELlYC04bXYowCP9GX47ivo2l+c= diff --git a/services/core/internal/cicdstate/store.go b/services/core/internal/cicdstate/store.go index 5aba4fc..a05bb7f 100644 --- a/services/core/internal/cicdstate/store.go +++ b/services/core/internal/cicdstate/store.go @@ -22,9 +22,9 @@ var validTransitions = map[string][]string{ StateCanceled: nil, } -// RunInput holds the remote run request payload stored on a job so that a -// runner can receive it when it claims the job. It deliberately avoids any -// protobuf dependency; conversion to/from otopb.RunRequest happens at the HTTP +// RunInput holds the remote run request payload stored on a job so that the +// runner transport can dispatch it later. It deliberately avoids any protobuf +// dependency; conversion to/from otopb.RunRequest happens at the HTTP or socket // boundary. type RunInput struct { PipelineYAMLPath string diff --git a/services/core/internal/httpserver/dto.go b/services/core/internal/httpserver/dto.go index ce19d2b..075baf1 100644 --- a/services/core/internal/httpserver/dto.go +++ b/services/core/internal/httpserver/dto.go @@ -27,7 +27,7 @@ func jobToJSON(j *cicdstate.Job) map[string]interface{} { return out } -// validateRunInput enforces the remote claim contract: exactly one of +// validateRunInput enforces the remote dispatch contract: exactly one of // pipeline_yaml or pipeline_yaml_path must be provided. func validateRunInput(in *cicdstate.RunInput) error { hasYAML := strings.TrimSpace(in.PipelineYAML) != "" @@ -59,8 +59,8 @@ func runRequestFromJSON(rr *otopb.RunRequest) *cicdstate.RunInput { return in } -// runRequestToProto builds the claim response RunRequest from the stored input, -// filling in the runner/job/execution identifiers resolved at claim time. +// runRequestToProto builds the runner-facing RunRequest from the stored input, +// filling in the runner/job/execution identifiers resolved at dispatch time. func runRequestToProto(in *cicdstate.RunInput, runnerID, jobID, execID string) *otopb.RunRequest { if in == nil { return nil diff --git a/services/core/internal/httpserver/job_handlers.go b/services/core/internal/httpserver/job_handlers.go index 495ea48..996620e 100644 --- a/services/core/internal/httpserver/job_handlers.go +++ b/services/core/internal/httpserver/job_handlers.go @@ -11,7 +11,7 @@ import ( otopb "github.com/toki/oto/services/core/oto" ) -func handleCreateJob(store *cicdstate.Store) http.HandlerFunc { +func handleCreateJob(store *cicdstate.Store, dispatcher runnerControlDispatcher) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { var req struct { ID string `json:"id"` @@ -31,6 +31,9 @@ func handleCreateJob(store *cicdstate.Store) http.HandlerFunc { writeResponse(w, http.StatusConflict, map[string]string{"error": err.Error()}) return } + if dispatcher != nil { + go dispatcher.DispatchQueuedJobs() + } writeResponse(w, http.StatusCreated, jobToJSON(job)) } } diff --git a/services/core/internal/httpserver/oto_agent_bootstrap.ps1 b/services/core/internal/httpserver/oto_agent_bootstrap.ps1 index 939ad3b..e6a691f 100644 --- a/services/core/internal/httpserver/oto_agent_bootstrap.ps1 +++ b/services/core/internal/httpserver/oto_agent_bootstrap.ps1 @@ -3,6 +3,7 @@ Set-StrictMode -Version Latest $ErrorActionPreference = 'Stop' $serverUrl = '' +$socketUrl = '' $agentId = '' $enrollmentToken = '' $releaseBaseUrl = '' @@ -18,6 +19,7 @@ while ($i -lt $args.Count) { switch ($args[$i]) { '--' { $i++ } '--server-url' { $serverUrl = $args[$i + 1]; $i += 2 } + '--socket-url' { $socketUrl = $args[$i + 1]; $i += 2 } '--agent-id' { $agentId = $args[$i + 1]; $i += 2 } '--enrollment-token' { $enrollmentToken = $args[$i + 1]; $i += 2 } '--release-base-url' { $releaseBaseUrl = $args[$i + 1]; $i += 2 } @@ -82,9 +84,14 @@ try { $agentAliasYaml = $agentAlias.Replace("'", "''") $enrollmentTokenYaml = $enrollmentToken.Replace("'", "''") $serverUrlYaml = $serverUrl.Replace("'", "''") + $socketUrlYaml = $socketUrl.Replace("'", "''") $installDirYaml = ($installDir -replace '\\', '/').Replace("'", "''") $workspaceRootYaml = ($workspaceRoot -replace '\\', '/').Replace("'", "''") $logDirYaml = ($logDir -replace '\\', '/').Replace("'", "''") + $socketUrlLine = '' + if ($socketUrl) { + $socketUrlLine = " socket_url: '$socketUrlYaml'`n" + } $configContent = @" agent: @@ -93,6 +100,7 @@ agent: enrollment_token: '$enrollmentTokenYaml' server: url: '$serverUrlYaml' +$socketUrlLine runtime: install_dir: '$installDirYaml' workspace_root: '$workspaceRootYaml' diff --git a/services/core/internal/httpserver/oto_agent_bootstrap.sh b/services/core/internal/httpserver/oto_agent_bootstrap.sh index 1d8468f..57af11a 100644 --- a/services/core/internal/httpserver/oto_agent_bootstrap.sh +++ b/services/core/internal/httpserver/oto_agent_bootstrap.sh @@ -2,6 +2,7 @@ set -euo pipefail server_url="" +socket_url="" agent_id="" enrollment_token="" release_base_url="" @@ -13,8 +14,9 @@ log_dir="${HOME}/.oto/agent/log" background="true" usage() { - echo "Usage: $0 --server-url --agent-id --enrollment-token --release-base-url [options]" + echo "Usage: $0 --server-url --socket-url --agent-id --enrollment-token --release-base-url [options]" echo "Options:" + echo " --socket-url Runner proto-socket URL (default: derived by agent)" echo " --agent-alias Alias for the agent" echo " --install-dir Directory to install OTO agent (default: \$HOME/.oto/bin)" echo " --config-path Path to config file (default: \$HOME/.oto/agent/config.yaml)" @@ -39,6 +41,10 @@ while [[ $# -gt 0 ]]; do server_url="$2" shift 2 ;; + --socket-url) + socket_url="$2" + shift 2 + ;; --agent-id) agent_id="$2" shift 2 @@ -155,6 +161,11 @@ agent: enrollment_token: "$(yaml_escape "$enrollment_token")" server: url: "$(yaml_escape "$server_url")" +EOF +if [[ -n "$socket_url" ]]; then + printf ' socket_url: "%s"\n' "$(yaml_escape "$socket_url")" >> "$config_path" +fi +cat <> "$config_path" runtime: install_dir: "$(yaml_escape "$install_dir")" workspace_root: "$(yaml_escape "$workspace_root")" diff --git a/services/core/internal/httpserver/routes.go b/services/core/internal/httpserver/routes.go index 17265cf..bc55b56 100644 --- a/services/core/internal/httpserver/routes.go +++ b/services/core/internal/httpserver/routes.go @@ -8,10 +8,15 @@ import ( "github.com/toki/oto/services/core/internal/runnerregistry" ) +type runnerControlDispatcher interface { + DispatchQueuedJobs() + CancelExecution(runnerID, execID, reason string) error +} + // registerRoutes registers all HTTP routes on the given ServeMux. // This file exists to separate route registration logic from handler // implementation, making it easier to refactor handlers and DTOs later. -func registerRoutes(mux *http.ServeMux, registry *runnerregistry.Registry, store *cicdstate.Store) { +func registerRoutes(mux *http.ServeMux, registry *runnerregistry.Registry, store *cicdstate.Store, dispatcher runnerControlDispatcher) { mux.HandleFunc("/healthz", withCORS(handleHealthz)) mux.HandleFunc("/readyz", withCORS(handleReadyz)) mux.HandleFunc("/api/v1/runners/register", withCORS(handleRunnerRegister(registry))) @@ -21,7 +26,7 @@ func registerRoutes(mux *http.ServeMux, registry *runnerregistry.Registry, store mux.HandleFunc("/api/v1/runners/{id}", withCORS(handleGetRunner(registry))) mux.HandleFunc("/bootstrap/oto-agent.sh", withCORS(handleServeBootstrapScript(embeddedBootstrapProvider{}))) mux.HandleFunc("/bootstrap/oto-agent.ps1", withCORS(handleServeBootstrapPs1(embeddedBootstrapPs1Provider{}))) - mux.HandleFunc("/api/v1/", withCORS(handleRouter(store, registry))) + mux.HandleFunc("/api/v1/", withCORS(handleRouterWithDispatcher(store, registry, dispatcher))) } func withCORS(next http.HandlerFunc) http.HandlerFunc { @@ -52,6 +57,10 @@ func apiPathSegments(path string) []string { } func handleRouter(store *cicdstate.Store, registry *runnerregistry.Registry) http.HandlerFunc { + return handleRouterWithDispatcher(store, registry, nil) +} + +func handleRouterWithDispatcher(store *cicdstate.Store, registry *runnerregistry.Registry, dispatcher runnerControlDispatcher) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { parts := apiPathSegments(r.URL.Path) if parts == nil { @@ -63,7 +72,7 @@ func handleRouter(store *cicdstate.Store, registry *runnerregistry.Registry) htt case "jobs": switch { case len(parts) == 1 && r.Method == http.MethodPost: - handleCreateJob(store)(w, r) + handleCreateJob(store, dispatcher)(w, r) case len(parts) == 1 && r.Method == http.MethodGet: writeResponse(w, http.StatusMethodNotAllowed, map[string]string{"error": "method not allowed"}) return @@ -109,7 +118,7 @@ func handleRouter(store *cicdstate.Store, registry *runnerregistry.Registry) htt case len(parts) == 4 && parts[2] == "jobs" && parts[3] == "claim" && r.Method == http.MethodPost: handleRunnerClaimJob(store, registry, runnerID)(w, r) case len(parts) == 5 && parts[2] == "executions" && parts[4] == "cancel" && r.Method == http.MethodPost: - handleRunnerCancelExecution(store, registry, runnerID, parts[3])(w, r) + handleRunnerCancelExecution(store, registry, dispatcher, runnerID, parts[3])(w, r) case len(parts) == 5 && parts[2] == "executions" && parts[4] == "report" && r.Method == http.MethodPost: handleRunnerReportExecution(store, registry, runnerID, parts[3])(w, r) case len(parts) == 5 && parts[2] == "executions" && parts[4] == "logs" && r.Method == http.MethodPost: diff --git a/services/core/internal/httpserver/runner_cicd_handlers.go b/services/core/internal/httpserver/runner_cicd_handlers.go index 1ff5713..7219cae 100644 --- a/services/core/internal/httpserver/runner_cicd_handlers.go +++ b/services/core/internal/httpserver/runner_cicd_handlers.go @@ -185,6 +185,14 @@ func handleRunnerReportExecution(store *cicdstate.Store, registry *runnerregistr }) return } + if exec.RunnerID != runnerID { + writeResponse(w, http.StatusBadRequest, &otopb.ExecutionReportResponse{ + RunnerId: runnerID, + ExecutionId: execID, + ErrorMessage: "execution owner mismatch", + }) + return + } jobID := strings.TrimSpace(req.GetJobId()) if jobID == "" { jobID = exec.JobID @@ -249,6 +257,12 @@ func handleRunnerAppendLog(store *cicdstate.Store, registry *runnerregistry.Regi }) return } + if err := ensureExecutionOwner(store, runnerID, execID); err != nil { + writeResponse(w, runnerExecutionErrorStatus(err), &otopb.LogAppendResponse{ + ErrorMessage: err.Error(), + }) + return + } line := strings.TrimSpace(req.GetLine()) if line == "" { writeResponse(w, http.StatusBadRequest, &otopb.LogAppendResponse{ @@ -283,6 +297,12 @@ func handleRunnerAppendArtifact(store *cicdstate.Store, registry *runnerregistry }) return } + if err := ensureExecutionOwner(store, runnerID, execID); err != nil { + writeResponse(w, runnerExecutionErrorStatus(err), &otopb.ArtifactReportResponse{ + ErrorMessage: err.Error(), + }) + return + } name := strings.TrimSpace(req.GetName()) path := strings.TrimSpace(req.GetPath()) if name == "" || path == "" { @@ -326,7 +346,25 @@ func ensureRunnerKnown(registry *runnerregistry.Registry, runnerID string) error return nil } -func handleRunnerCancelExecution(store *cicdstate.Store, registry *runnerregistry.Registry, runnerID string, execID string) http.HandlerFunc { +func ensureExecutionOwner(store *cicdstate.Store, runnerID string, execID string) error { + exec, err := store.GetExecution(execID) + if err != nil { + return fmt.Errorf("execution not found") + } + if exec.RunnerID != runnerID { + return fmt.Errorf("execution owner mismatch") + } + return nil +} + +func runnerExecutionErrorStatus(err error) int { + if err != nil && err.Error() == "execution not found" { + return http.StatusNotFound + } + return http.StatusBadRequest +} + +func handleRunnerCancelExecution(store *cicdstate.Store, registry *runnerregistry.Registry, dispatcher runnerControlDispatcher, runnerID string, execID string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { var req otopb.CancelRunRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -370,6 +408,10 @@ func handleRunnerCancelExecution(store *cicdstate.Store, registry *runnerregistr return } + if dispatcher != nil { + _ = dispatcher.CancelExecution(runnerID, execID, req.GetReason()) + } + writeResponse(w, http.StatusOK, &otopb.CancelRunResponse{ Success: true, }) diff --git a/services/core/internal/httpserver/runner_handlers.go b/services/core/internal/httpserver/runner_handlers.go index f03493b..5578ca8 100644 --- a/services/core/internal/httpserver/runner_handlers.go +++ b/services/core/internal/httpserver/runner_handlers.go @@ -3,6 +3,7 @@ package httpserver import ( "encoding/json" "fmt" + "net" "net/http" "net/url" "os" @@ -242,29 +243,36 @@ func handleRunnerBootstrapCommand(registry *runnerregistry.Registry) http.Handle return } } + socketURL, err := runnerSocketPublicURL(serverURL) + if err != nil { + writeResponse(w, http.StatusBadRequest, errorToJSON(err.Error())) + return + } var bootstrapCmd string if target == "windows" { escapedScriptURL := powershellEscape(serverURL + "/bootstrap/oto-agent.ps1") escapedServerURL := powershellEscape(serverURL) + escapedSocketURL := powershellEscape(socketURL) escapedRunnerID := powershellEscape(runnerID) escapedToken := powershellEscape(token) escapedReleaseURL := powershellEscape(releaseBaseURL) bootstrapCmd = fmt.Sprintf( - "powershell -ExecutionPolicy Bypass -Command \"& ([scriptblock]::Create((irm %s))) -- --server-url %s --agent-id %s --enrollment-token %s --release-base-url %s\"", - escapedScriptURL, escapedServerURL, escapedRunnerID, escapedToken, escapedReleaseURL, + "powershell -ExecutionPolicy Bypass -Command \"& ([scriptblock]::Create((irm %s))) -- --server-url %s --socket-url %s --agent-id %s --enrollment-token %s --release-base-url %s\"", + escapedScriptURL, escapedServerURL, escapedSocketURL, escapedRunnerID, escapedToken, escapedReleaseURL, ) } else { escapedScriptURL := shellEscape(serverURL + "/bootstrap/oto-agent.sh") escapedServerURL := shellEscape(serverURL) + escapedSocketURL := shellEscape(socketURL) escapedRunnerID := shellEscape(runnerID) escapedToken := shellEscape(token) escapedReleaseURL := shellEscape(releaseBaseURL) bootstrapCmd = fmt.Sprintf( - "curl -fsSL %s | bash -s -- --server-url %s --agent-id %s --enrollment-token %s --release-base-url %s", - escapedScriptURL, escapedServerURL, escapedRunnerID, escapedToken, escapedReleaseURL, + "curl -fsSL %s | bash -s -- --server-url %s --socket-url %s --agent-id %s --enrollment-token %s --release-base-url %s", + escapedScriptURL, escapedServerURL, escapedSocketURL, escapedRunnerID, escapedToken, escapedReleaseURL, ) } @@ -276,6 +284,24 @@ func handleRunnerBootstrapCommand(registry *runnerregistry.Registry) http.Handle } } +func runnerSocketPublicURL(serverURL string) (string, error) { + if configured := strings.TrimSpace(os.Getenv("OTO_RUNNER_SOCKET_PUBLIC_URL")); configured != "" { + if !strings.Contains(configured, "://") { + configured = "tcp://" + configured + } + u, err := url.Parse(configured) + if err != nil || u.Hostname() == "" { + return "", fmt.Errorf("invalid OTO_RUNNER_SOCKET_PUBLIC_URL") + } + return configured, nil + } + u, err := url.Parse(serverURL) + if err != nil || u.Hostname() == "" { + return "", fmt.Errorf("invalid server URL") + } + return "tcp://" + net.JoinHostPort(u.Hostname(), "18080"), nil +} + func handleServeBootstrapScript(provider bootstrapScriptProvider) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { diff --git a/services/core/internal/httpserver/server.go b/services/core/internal/httpserver/server.go index d71f273..9333110 100644 --- a/services/core/internal/httpserver/server.go +++ b/services/core/internal/httpserver/server.go @@ -9,6 +9,7 @@ import ( "github.com/toki/oto/services/core/internal/cicdstate" "github.com/toki/oto/services/core/internal/runnerregistry" + "github.com/toki/oto/services/core/internal/runnersocket" ) const ( @@ -26,13 +27,16 @@ type ServerConfig struct { type Server struct { httpServer *http.Server registry *runnerregistry.Registry + store *cicdstate.Store heartbeatTimeout time.Duration scanInterval time.Duration + runnerSocket *runnersocket.Server - mu sync.Mutex - loopCancel context.CancelFunc - loopDone <-chan struct{} - startOnce sync.Once + mu sync.Mutex + loopCancel context.CancelFunc + loopDone <-chan struct{} + socketCancel context.CancelFunc + startOnce sync.Once } // NewServer creates a new instance of Server. @@ -57,17 +61,31 @@ func NewServerWithConfig(addr string, registry *runnerregistry.Registry, store * func newServer(addr string, registry *runnerregistry.Registry, store *cicdstate.Store, heartbeatTimeout, scanInterval time.Duration) *Server { mux := http.NewServeMux() - registerRoutes(mux, registry, store) - return &Server{ + s := &Server{ httpServer: &http.Server{ Addr: addr, Handler: mux, }, registry: registry, + store: store, heartbeatTimeout: heartbeatTimeout, scanInterval: scanInterval, } + registerRoutes(mux, registry, store, s) + return s +} + +// EnableRunnerSocket starts a runner-facing proto-socket listener together with +// the HTTP server. The listener shares the same registry and CICD store. +func (s *Server) EnableRunnerSocket(addr string) { + s.mu.Lock() + defer s.mu.Unlock() + if addr == "" { + s.runnerSocket = nil + return + } + s.runnerSocket = runnersocket.New(addr, s.registry, s.store) } func (s *Server) startTimeoutLoop() { @@ -108,10 +126,63 @@ func (s *Server) stopTimeoutLoop() { } } +func (s *Server) startRunnerSocket() error { + s.mu.Lock() + runnerSocket := s.runnerSocket + if runnerSocket == nil { + s.mu.Unlock() + return nil + } + ctx, cancel := context.WithCancel(context.Background()) + s.socketCancel = cancel + s.mu.Unlock() + return runnerSocket.Start(ctx) +} + +func (s *Server) stopRunnerSocket() { + s.mu.Lock() + cancel := s.socketCancel + runnerSocket := s.runnerSocket + s.socketCancel = nil + s.mu.Unlock() + if cancel != nil { + cancel() + } + if runnerSocket != nil { + _ = runnerSocket.Stop() + } +} + +// DispatchQueuedJobs lets HTTP handlers notify the runner socket dispatcher. +func (s *Server) DispatchQueuedJobs() { + s.mu.Lock() + runnerSocket := s.runnerSocket + s.mu.Unlock() + if runnerSocket != nil { + runnerSocket.DispatchQueuedJobs() + } +} + +// CancelExecution forwards a cancellation request to a connected runner socket. +func (s *Server) CancelExecution(runnerID, execID, reason string) error { + s.mu.Lock() + runnerSocket := s.runnerSocket + s.mu.Unlock() + if runnerSocket == nil { + return nil + } + return runnerSocket.CancelExecution(runnerID, execID, reason) +} + // Start starts the HTTP server. func (s *Server) Start() error { s.startOnce.Do(s.startTimeoutLoop) + if err := s.startRunnerSocket(); err != nil { + s.stopTimeoutLoop() + return err + } err := s.httpServer.ListenAndServe() + s.stopRunnerSocket() s.stopTimeoutLoop() return err } @@ -120,13 +191,19 @@ func (s *Server) Start() error { // Useful for testing with dynamic ports. func (s *Server) StartListener(ln net.Listener) error { s.startOnce.Do(s.startTimeoutLoop) + if err := s.startRunnerSocket(); err != nil { + s.stopTimeoutLoop() + return err + } err := s.httpServer.Serve(ln) + s.stopRunnerSocket() s.stopTimeoutLoop() return err } // Shutdown gracefully shuts down the server. func (s *Server) Shutdown(ctx context.Context) error { + s.stopRunnerSocket() s.stopTimeoutLoop() return s.httpServer.Shutdown(ctx) } diff --git a/services/core/internal/httpserver/server_test.go b/services/core/internal/httpserver/server_test.go index e545539..31d1408 100644 --- a/services/core/internal/httpserver/server_test.go +++ b/services/core/internal/httpserver/server_test.go @@ -479,7 +479,7 @@ func TestHandleRunnerBootstrapCommand(t *testing.T) { t.Fatalf("decode bootstrap command response: %v", err) } - expectedCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'" + expectedCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --socket-url 'tcp://localhost:18080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'" if response.GetBootstrapCommand() != expectedCmd { t.Fatalf("bootstrap_command = %q, want %q", response.GetBootstrapCommand(), expectedCmd) } @@ -499,7 +499,7 @@ func TestHandleRunnerBootstrapCommand(t *testing.T) { t.Fatalf("decode bootstrap command response: %v", err) } - expectedEscapedCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --agent-id 'runner; rm -rf /' --enrollment-token 'token'\\''$(say hello)'\\''' --release-base-url 'https://example.com/releases'" + expectedEscapedCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --socket-url 'tcp://localhost:18080' --agent-id 'runner; rm -rf /' --enrollment-token 'token'\\''$(say hello)'\\''' --release-base-url 'https://example.com/releases'" if response.GetBootstrapCommand() != expectedEscapedCmd { t.Fatalf("bootstrap_command = %q, want %q", response.GetBootstrapCommand(), expectedEscapedCmd) } @@ -539,7 +539,7 @@ func TestHandleRunnerBootstrapCommand(t *testing.T) { if err := json.Unmarshal(rr.Body.Bytes(), &response); err != nil { t.Fatalf("decode bootstrap command response: %v", err) } - expectedLinuxCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'" + expectedLinuxCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --socket-url 'tcp://localhost:18080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'" if response.GetBootstrapCommand() != expectedLinuxCmd { t.Fatalf("bootstrap_command for linux = %q, want %q", response.GetBootstrapCommand(), expectedLinuxCmd) } @@ -557,7 +557,7 @@ func TestHandleRunnerBootstrapCommand(t *testing.T) { if err := json.Unmarshal(rr.Body.Bytes(), &response); err != nil { t.Fatalf("decode bootstrap command response: %v", err) } - expectedMacosCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'" + expectedMacosCmd := "curl -fsSL 'http://localhost:8080/bootstrap/oto-agent.sh' | bash -s -- --server-url 'http://localhost:8080' --socket-url 'tcp://localhost:18080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'" if response.GetBootstrapCommand() != expectedMacosCmd { t.Fatalf("bootstrap_command for macos = %q, want %q", response.GetBootstrapCommand(), expectedMacosCmd) } @@ -575,7 +575,7 @@ func TestHandleRunnerBootstrapCommand(t *testing.T) { if err := json.Unmarshal(rr.Body.Bytes(), &response); err != nil { t.Fatalf("decode bootstrap command response: %v", err) } - expectedWindowsCmd := "powershell -ExecutionPolicy Bypass -Command \"& ([scriptblock]::Create((irm 'http://localhost:8080/bootstrap/oto-agent.ps1'))) -- --server-url 'http://localhost:8080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'\"" + expectedWindowsCmd := "powershell -ExecutionPolicy Bypass -Command \"& ([scriptblock]::Create((irm 'http://localhost:8080/bootstrap/oto-agent.ps1'))) -- --server-url 'http://localhost:8080' --socket-url 'tcp://localhost:18080' --agent-id 'runner-123' --enrollment-token 'token-123' --release-base-url 'https://example.com/releases'\"" if response.GetBootstrapCommand() != expectedWindowsCmd { t.Fatalf("bootstrap_command for windows = %q, want %q", response.GetBootstrapCommand(), expectedWindowsCmd) } @@ -593,7 +593,7 @@ func TestHandleRunnerBootstrapCommand(t *testing.T) { if err := json.Unmarshal(rr.Body.Bytes(), &response); err != nil { t.Fatalf("decode bootstrap command response: %v", err) } - expectedWindowsEscapedCmd := "powershell -ExecutionPolicy Bypass -Command \"& ([scriptblock]::Create((irm 'http://localhost:8080/bootstrap/oto-agent.ps1'))) -- --server-url 'http://localhost:8080' --agent-id 'runner; rm -rf /' --enrollment-token 'token''$(say hello)''' --release-base-url 'https://example.com/releases'\"" + expectedWindowsEscapedCmd := "powershell -ExecutionPolicy Bypass -Command \"& ([scriptblock]::Create((irm 'http://localhost:8080/bootstrap/oto-agent.ps1'))) -- --server-url 'http://localhost:8080' --socket-url 'tcp://localhost:18080' --agent-id 'runner; rm -rf /' --enrollment-token 'token''$(say hello)''' --release-base-url 'https://example.com/releases'\"" if response.GetBootstrapCommand() != expectedWindowsEscapedCmd { t.Fatalf("bootstrap_command for windows malicious = %q, want %q", response.GetBootstrapCommand(), expectedWindowsEscapedCmd) } diff --git a/services/core/internal/runnersocket/server.go b/services/core/internal/runnersocket/server.go new file mode 100644 index 0000000..f6140f7 --- /dev/null +++ b/services/core/internal/runnersocket/server.go @@ -0,0 +1,448 @@ +package runnersocket + +import ( + "context" + "fmt" + "net" + "sort" + "strconv" + "strings" + "sync" + + protoSocket "git.toki-labs.com/toki/proto-socket/go" + "github.com/toki/oto/services/core/internal/cicdstate" + "github.com/toki/oto/services/core/internal/runnerregistry" + otopb "github.com/toki/oto/services/core/oto" + "google.golang.org/protobuf/proto" +) + +const ( + heartbeatIntervalSeconds = 30 + heartbeatWaitSeconds = 45 +) + +// Server owns the runner-facing proto-socket listener. +type Server struct { + addr string + registry *runnerregistry.Registry + store *cicdstate.Store + + mu sync.RWMutex + server *protoSocket.TcpServer + clients map[string]*protoSocket.TcpClient +} + +// New creates a runner socket server sharing the core registry and CICD store. +func New(addr string, registry *runnerregistry.Registry, store *cicdstate.Store) *Server { + return &Server{ + addr: addr, + registry: registry, + store: store, + clients: make(map[string]*protoSocket.TcpClient), + } +} + +// Start binds the proto-socket TCP listener. +func (s *Server) Start(ctx context.Context) error { + host, port, err := splitAddr(s.addr) + if err != nil { + return err + } + server := protoSocket.NewTcpServer(host, port, func(conn net.Conn) *protoSocket.TcpClient { + client := protoSocket.NewTcpClient(conn, heartbeatIntervalSeconds, heartbeatWaitSeconds, parserMap()) + s.attachClientHandlers(client) + return client + }) + + s.mu.Lock() + s.server = server + s.mu.Unlock() + + return server.Start(ctx) +} + +// Stop closes the listener and connected runner sockets. +func (s *Server) Stop() error { + s.mu.RLock() + server := s.server + s.mu.RUnlock() + if server == nil { + return nil + } + return server.Stop() +} + +// DispatchQueuedJobs pushes queued jobs to currently connected idle runners. +func (s *Server) DispatchQueuedJobs() { + for _, runnerID := range s.connectedRunnerIDs() { + if s.runnerHasActiveExecution(runnerID) { + continue + } + client := s.clientFor(runnerID) + if client == nil { + continue + } + runRequest, jobID, execID, err := s.claimNextQueuedJob(runnerID) + if err != nil { + if jobID != "" { + if execID != "" { + _ = s.store.AppendLog(execID, fmt.Sprintf("failed to prepare run request over proto-socket: %v", err)) + _ = s.store.TransitionExecution(execID, cicdstate.StateFailed) + } + _ = s.store.TransitionJob(jobID, cicdstate.StateFailed) + } + continue + } + if runRequest == nil { + continue + } + if err := client.Send(runRequest); err != nil { + _ = s.store.AppendLog(execID, fmt.Sprintf("failed to dispatch run request over proto-socket: %v", err)) + _ = s.store.TransitionExecution(execID, cicdstate.StateFailed) + _ = s.store.TransitionJob(jobID, cicdstate.StateFailed) + continue + } + } +} + +// CancelExecution asks a connected runner to cancel an execution. +func (s *Server) CancelExecution(runnerID, execID, reason string) error { + client := s.clientFor(runnerID) + if client == nil { + return fmt.Errorf("runner socket is not connected") + } + return client.Send(&otopb.CancelRunRequest{ + RunnerId: runnerID, + ExecutionId: execID, + Reason: reason, + }) +} + +func (s *Server) attachClientHandlers(client *protoSocket.TcpClient) { + var runnerIDMu sync.RWMutex + runnerID := "" + + setRunnerID := func(id string) { + runnerIDMu.Lock() + runnerID = id + runnerIDMu.Unlock() + } + getRunnerID := func() string { + runnerIDMu.RLock() + defer runnerIDMu.RUnlock() + return runnerID + } + + protoSocket.AddRequestListenerTyped[*otopb.RegisterRunnerRequest, *otopb.RegisterRunnerResponse]( + &client.Communicator, + func(req *otopb.RegisterRunnerRequest) (*otopb.RegisterRunnerResponse, error) { + res := s.registry.Register(req) + if res.GetAccepted() { + id := res.GetRunnerId() + setRunnerID(id) + s.setClient(id, client) + s.registry.Heartbeat(id, otopb.HeartbeatStatus_HEARTBEAT_STATUS_HEALTHY) + go s.DispatchQueuedJobs() + } + return res, nil + }, + ) + + protoSocket.AddRequestListenerTyped[*otopb.HeartbeatRequest, *otopb.HeartbeatResponse]( + &client.Communicator, + func(req *otopb.HeartbeatRequest) (*otopb.HeartbeatResponse, error) { + runnerID := strings.TrimSpace(req.GetRunnerId()) + if runnerID == "" { + runnerID = getRunnerID() + } + return s.registry.Heartbeat(runnerID, req.GetStatus()), nil + }, + ) + + protoSocket.AddRequestListenerTyped[*otopb.ExecutionReportRequest, *otopb.ExecutionReportResponse]( + &client.Communicator, + func(req *otopb.ExecutionReportRequest) (*otopb.ExecutionReportResponse, error) { + res := s.handleExecutionReport(req) + if res.GetAccepted() { + go s.DispatchQueuedJobs() + } + return res, nil + }, + ) + + protoSocket.AddRequestListenerTyped[*otopb.LogAppendRequest, *otopb.LogAppendResponse]( + &client.Communicator, + func(req *otopb.LogAppendRequest) (*otopb.LogAppendResponse, error) { + return s.handleLogAppend(req), nil + }, + ) + + protoSocket.AddRequestListenerTyped[*otopb.ArtifactReportRequest, *otopb.ArtifactReportResponse]( + &client.Communicator, + func(req *otopb.ArtifactReportRequest) (*otopb.ArtifactReportResponse, error) { + return s.handleArtifactReport(req), nil + }, + ) + + client.AddDisconnectListener(func(c *protoSocket.TcpClient) { + id := getRunnerID() + if id == "" { + return + } + if s.removeClient(id, c) { + s.registry.Disconnect(id) + } + }) +} + +func (s *Server) handleExecutionReport(req *otopb.ExecutionReportRequest) *otopb.ExecutionReportResponse { + runnerID := strings.TrimSpace(req.GetRunnerId()) + execID := strings.TrimSpace(req.GetExecutionId()) + if err := s.ensureRunnerKnown(runnerID); err != nil { + return &otopb.ExecutionReportResponse{RunnerId: runnerID, ExecutionId: execID, ErrorMessage: err.Error()} + } + exec, err := s.store.GetExecution(execID) + if err != nil { + return &otopb.ExecutionReportResponse{RunnerId: runnerID, ExecutionId: execID, ErrorMessage: "execution not found"} + } + if exec.RunnerID != runnerID { + return &otopb.ExecutionReportResponse{RunnerId: runnerID, ExecutionId: execID, ErrorMessage: "execution owner mismatch"} + } + jobID := strings.TrimSpace(req.GetJobId()) + if jobID == "" { + jobID = exec.JobID + } else if jobID != exec.JobID { + return &otopb.ExecutionReportResponse{RunnerId: runnerID, JobId: jobID, ExecutionId: execID, ErrorMessage: "job id mismatch for execution"} + } + + targetState := cicdstate.StateFailed + if req.GetSuccess() { + targetState = cicdstate.StateSucceeded + } + if err := s.store.TransitionExecution(execID, targetState); err != nil { + return &otopb.ExecutionReportResponse{RunnerId: runnerID, JobId: jobID, ExecutionId: execID, ErrorMessage: err.Error()} + } + if err := s.store.TransitionJob(jobID, targetState); err != nil { + return &otopb.ExecutionReportResponse{RunnerId: runnerID, JobId: jobID, ExecutionId: execID, ErrorMessage: err.Error()} + } + if strings.TrimSpace(req.GetMessage()) != "" { + _ = s.store.AppendLog(execID, req.GetMessage()) + } + return &otopb.ExecutionReportResponse{ + Accepted: true, + RunnerId: runnerID, + JobId: jobID, + ExecutionId: execID, + State: targetState, + } +} + +func (s *Server) handleLogAppend(req *otopb.LogAppendRequest) *otopb.LogAppendResponse { + runnerID := strings.TrimSpace(req.GetRunnerId()) + execID := strings.TrimSpace(req.GetExecutionId()) + if err := s.ensureRunnerKnown(runnerID); err != nil { + return &otopb.LogAppendResponse{ErrorMessage: err.Error()} + } + if err := s.ensureExecutionOwner(runnerID, execID); err != nil { + return &otopb.LogAppendResponse{ErrorMessage: err.Error()} + } + line := strings.TrimSpace(req.GetLine()) + if line == "" { + return &otopb.LogAppendResponse{ErrorMessage: "line is required"} + } + if err := s.store.AppendLog(execID, line); err != nil { + return &otopb.LogAppendResponse{ErrorMessage: "execution not found"} + } + return &otopb.LogAppendResponse{Accepted: true} +} + +func (s *Server) handleArtifactReport(req *otopb.ArtifactReportRequest) *otopb.ArtifactReportResponse { + runnerID := strings.TrimSpace(req.GetRunnerId()) + execID := strings.TrimSpace(req.GetExecutionId()) + if err := s.ensureRunnerKnown(runnerID); err != nil { + return &otopb.ArtifactReportResponse{ErrorMessage: err.Error()} + } + if err := s.ensureExecutionOwner(runnerID, execID); err != nil { + return &otopb.ArtifactReportResponse{ErrorMessage: err.Error()} + } + name := strings.TrimSpace(req.GetName()) + path := strings.TrimSpace(req.GetPath()) + if name == "" || path == "" { + return &otopb.ArtifactReportResponse{ErrorMessage: "name and path are required"} + } + if err := s.store.AppendArtifact(execID, name, path); err != nil { + return &otopb.ArtifactReportResponse{ErrorMessage: "execution not found"} + } + return &otopb.ArtifactReportResponse{Accepted: true} +} + +func (s *Server) claimNextQueuedJob(runnerID string) (*otopb.RunRequest, string, string, error) { + if err := s.ensureRunnerKnown(runnerID); err != nil { + return nil, "", "", err + } + var foundJob *cicdstate.Job + for _, job := range s.store.SnapshotJobs() { + if job.State != cicdstate.StateQueued { + continue + } + if foundJob == nil || job.CreatedAt.Before(foundJob.CreatedAt) { + copyJob := job + foundJob = ©Job + } + } + if foundJob == nil { + return nil, "", "", nil + } + if foundJob.RunInput == nil { + return nil, foundJob.ID, "", fmt.Errorf("job has no run request") + } + if err := validateRunInput(foundJob.RunInput); err != nil { + return nil, foundJob.ID, "", err + } + + execID := s.store.NextExecutionID() + if _, err := s.store.CreateExecution(foundJob.ID, execID); err != nil { + return nil, foundJob.ID, execID, err + } + _ = s.store.SetExecutionRunnerID(execID, runnerID) + if err := s.store.TransitionJob(foundJob.ID, cicdstate.StateRunning); err != nil { + return nil, foundJob.ID, execID, err + } + if err := s.store.TransitionExecution(execID, cicdstate.StateRunning); err != nil { + return nil, foundJob.ID, execID, err + } + return runRequestToProto(foundJob.RunInput, runnerID, foundJob.ID, execID), foundJob.ID, execID, nil +} + +func (s *Server) runnerHasActiveExecution(runnerID string) bool { + for _, exec := range s.store.SnapshotExecutions() { + if exec.RunnerID == runnerID && exec.State == cicdstate.StateRunning { + return true + } + } + return false +} + +func (s *Server) ensureRunnerKnown(runnerID string) error { + if strings.TrimSpace(runnerID) == "" { + return fmt.Errorf("missing runner id") + } + if s.registry == nil { + return nil + } + if _, ok := s.registry.Snapshot(runnerID); !ok { + return fmt.Errorf("runner not found") + } + return nil +} + +func (s *Server) ensureExecutionOwner(runnerID, execID string) error { + exec, err := s.store.GetExecution(execID) + if err != nil { + return fmt.Errorf("execution not found") + } + if exec.RunnerID != runnerID { + return fmt.Errorf("execution owner mismatch") + } + return nil +} + +func (s *Server) setClient(runnerID string, client *protoSocket.TcpClient) { + s.mu.Lock() + old := s.clients[runnerID] + s.clients[runnerID] = client + s.mu.Unlock() + + // Close outside the lock: Close() fires disconnect listeners synchronously, + // and those listeners call removeClient which also acquires s.mu. Closing + // inside the lock would cause a deadlock on re-entry. + if old != nil && old != client { + _ = old.Close() + } +} + +func (s *Server) removeClient(runnerID string, client *protoSocket.TcpClient) bool { + s.mu.Lock() + defer s.mu.Unlock() + if s.clients[runnerID] == client { + delete(s.clients, runnerID) + return true + } + return false +} + +func (s *Server) clientFor(runnerID string) *protoSocket.TcpClient { + s.mu.RLock() + defer s.mu.RUnlock() + return s.clients[runnerID] +} + +func (s *Server) connectedRunnerIDs() []string { + s.mu.RLock() + defer s.mu.RUnlock() + ids := make([]string, 0, len(s.clients)) + for id := range s.clients { + ids = append(ids, id) + } + sort.Strings(ids) + return ids +} + +func parserMap() protoSocket.ParserMap { + return protoSocket.ParserMap{ + protoSocket.TypeNameOf(&otopb.RegisterRunnerRequest{}): parserFor(func() *otopb.RegisterRunnerRequest { return &otopb.RegisterRunnerRequest{} }), + protoSocket.TypeNameOf(&otopb.HeartbeatRequest{}): parserFor(func() *otopb.HeartbeatRequest { return &otopb.HeartbeatRequest{} }), + protoSocket.TypeNameOf(&otopb.ExecutionReportRequest{}): parserFor(func() *otopb.ExecutionReportRequest { return &otopb.ExecutionReportRequest{} }), + protoSocket.TypeNameOf(&otopb.LogAppendRequest{}): parserFor(func() *otopb.LogAppendRequest { return &otopb.LogAppendRequest{} }), + protoSocket.TypeNameOf(&otopb.ArtifactReportRequest{}): parserFor(func() *otopb.ArtifactReportRequest { return &otopb.ArtifactReportRequest{} }), + } +} + +func parserFor[T proto.Message](newMessage func() T) func([]byte) (proto.Message, error) { + return func(b []byte) (proto.Message, error) { + m := newMessage() + return m, proto.Unmarshal(b, m) + } +} + +func splitAddr(addr string) (string, int, error) { + host, portText, err := net.SplitHostPort(addr) + if err != nil { + return "", 0, err + } + port, err := strconv.Atoi(portText) + if err != nil { + return "", 0, err + } + return host, port, nil +} + +func validateRunInput(in *cicdstate.RunInput) error { + hasYAML := strings.TrimSpace(in.PipelineYAML) != "" + hasPath := strings.TrimSpace(in.PipelineYAMLPath) != "" + if hasYAML == hasPath { + return fmt.Errorf("run request must set exactly one of pipeline_yaml or pipeline_yaml_path") + } + return nil +} + +func runRequestToProto(in *cicdstate.RunInput, runnerID, jobID, execID string) *otopb.RunRequest { + if in == nil { + return nil + } + rr := &otopb.RunRequest{ + RunnerId: runnerID, + JobId: jobID, + ExecutionId: execID, + PipelineYamlPath: in.PipelineYAMLPath, + PipelineYaml: in.PipelineYAML, + CommandTypes: append([]string(nil), in.CommandTypes...), + } + if len(in.Variables) > 0 { + rr.Variables = make(map[string]string, len(in.Variables)) + for k, v := range in.Variables { + rr.Variables[k] = v + } + } + return rr +} diff --git a/services/core/internal/runnersocket/server_test.go b/services/core/internal/runnersocket/server_test.go new file mode 100644 index 0000000..e664eee --- /dev/null +++ b/services/core/internal/runnersocket/server_test.go @@ -0,0 +1,207 @@ +package runnersocket + +import ( + "net" + "testing" + "time" + + protoSocket "git.toki-labs.com/toki/proto-socket/go" + "github.com/toki/oto/services/core/internal/cicdstate" + "github.com/toki/oto/services/core/internal/runnerregistry" + otopb "github.com/toki/oto/services/core/oto" +) + +func testServer(registry *runnerregistry.Registry) *Server { + return New("127.0.0.1:0", registry, cicdstate.NewStore()) +} + +// pipeClient creates a TcpClient backed by one end of a net.Pipe. +// heartbeat is disabled (intervalSec=0) to avoid background timer interference. +// Cleanup closes both ends. +func pipeClient(t *testing.T) (*protoSocket.TcpClient, net.Conn) { + t.Helper() + a, b := net.Pipe() + client := protoSocket.NewTcpClient(a, 0, 0, parserMap()) + t.Cleanup(func() { _ = a.Close(); _ = b.Close() }) + return client, b +} + +// TestServerSetClientReplacesDuplicate verifies that registering a second client +// for the same runner ID closes the old one and stores the new one. +func TestServerReplacesDuplicateRunnerClient(t *testing.T) { + s := testServer(runnerregistry.New()) + + client1, _ := pipeClient(t) + client2, _ := pipeClient(t) + + s.setClient("runner-A", client1) + s.setClient("runner-A", client2) + + // Allow Close() to propagate through connCloseOnce. + time.Sleep(20 * time.Millisecond) + + if client1.IsAlive() { + t.Error("old client must be closed after replacement, but it is still alive") + } + if got := s.clientFor("runner-A"); got != client2 { + t.Errorf("clientFor must return new client after replacement; got %v", got) + } +} + +// TestServerIgnoresStaleClientDisconnect verifies that removeClient ignores +// a client that is not the current entry for the given runner ID. +func TestServerIgnoresStaleClientDisconnect(t *testing.T) { + s := testServer(runnerregistry.New()) + + client1, _ := pipeClient(t) + client2, _ := pipeClient(t) + + s.setClient("runner-B", client1) + + // client2 is a different object – simulates a stale disconnect listener. + removed := s.removeClient("runner-B", client2) + if removed { + t.Error("removeClient must return false for a client that is not the current one") + } + if got := s.clientFor("runner-B"); got != client1 { + t.Errorf("current client must still be client1 after stale removeClient; got %v", got) + } +} + +// TestServerStaleClientDisconnectDoesNotDisconnectRegistry verifies the complete +// stale-disconnect protection chain: +// +// 1. setClient replaces the old client. +// 2. removeClient returns false for the stale old client. +// 3. Because removeClient returns false, registry.Disconnect is NOT called, +// so the runner remains in a healthy (non-disconnected) state. +func TestServerStaleClientDisconnectDoesNotDisconnectRegistry(t *testing.T) { + registry := runnerregistry.New() + s := testServer(registry) + + resp := registry.Register(&otopb.RegisterRunnerRequest{ + EnrollmentToken: "tok", + RunnerId: "runner-C", + ProtocolVersion: "oto.runner.v1", + Capability: &otopb.RunnerCapability{Name: "oto-runner", Version: "1.0.0"}, + }) + if !resp.GetAccepted() { + t.Fatalf("registry.Register rejected: %s", resp.GetRejectReason()) + } + registry.Heartbeat("runner-C", otopb.HeartbeatStatus_HEARTBEAT_STATUS_HEALTHY) + + client1, _ := pipeClient(t) + client2, _ := pipeClient(t) + + s.setClient("runner-C", client1) + // Duplicate connection: client2 replaces client1. + s.setClient("runner-C", client2) + + // Simulate stale client1's disconnect listener. + removed := s.removeClient("runner-C", client1) + if removed { + t.Error("stale removeClient should return false") + } + + // Because removeClient returned false the disconnect listener guard skips + // registry.Disconnect. The runner must NOT be in disconnected state. + runner, ok := registry.Snapshot("runner-C") + if !ok { + t.Fatal("runner-C not found in registry") + } + if runner.Status == runnerregistry.StatusDisconnected { + t.Errorf("runner status must not be %q after stale disconnect; got %q", + runnerregistry.StatusDisconnected, runner.Status) + } + if got := s.clientFor("runner-C"); got != client2 { + t.Errorf("current client must still be client2; got %v", got) + } +} + +// TestServerDuplicateReplaceWithListenerDoesNotDeadlock verifies that replacing +// a client that carries a production-style disconnect listener does not deadlock. +// +// Before the fix, setClient held s.mu while calling old.Close(), which fired +// the listener synchronously. The listener called removeClient → tried to +// acquire s.mu again → deadlock. After the fix, old.Close() is called after +// s.mu is released, so the re-entry is safe. +func TestServerDuplicateReplaceWithListenerDoesNotDeadlock(t *testing.T) { + registry := runnerregistry.New() + s := testServer(registry) + + resp := registry.Register(&otopb.RegisterRunnerRequest{ + EnrollmentToken: "tok", + RunnerId: "runner-E", + ProtocolVersion: "oto.runner.v1", + Capability: &otopb.RunnerCapability{Name: "oto-runner", Version: "1.0.0"}, + }) + if !resp.GetAccepted() { + t.Fatalf("registry.Register rejected: %s", resp.GetRejectReason()) + } + + client1, _ := pipeClient(t) + client2, _ := pipeClient(t) + + // Mirror the production disconnect listener from attachClientHandlers. + // This is the re-entry path: removeClient acquires s.mu, which would + // deadlock if setClient still held s.mu during old.Close(). + client1.AddDisconnectListener(func(c *protoSocket.TcpClient) { + if s.removeClient("runner-E", c) { + s.registry.Disconnect("runner-E") + } + }) + + s.setClient("runner-E", client1) + + // This must complete without hanging. If the lock-order bug is present the + // goroutine stalls here and the test times out via go test -timeout. + s.setClient("runner-E", client2) + + // Allow Close() and the disconnect listener to propagate. + time.Sleep(20 * time.Millisecond) + + if client1.IsAlive() { + t.Error("old client must be closed after replacement") + } + if got := s.clientFor("runner-E"); got != client2 { + t.Errorf("clientFor must return new client after replacement; got %v", got) + } + // The stale listener must not have disconnected the runner. + runner, ok := registry.Snapshot("runner-E") + if !ok { + t.Fatal("runner-E not found in registry after replacement") + } + if runner.Status == runnerregistry.StatusDisconnected { + t.Errorf("runner must not be disconnected after stale listener fires; got %q", runner.Status) + } +} + +// TestServerHeartbeatUsesRegisteredRunnerID verifies that connectedRunnerIDs +// reflects only currently active clients and that clientFor returns nil after +// the current client is removed. +func TestServerHeartbeatUsesRegisteredRunnerID(t *testing.T) { + s := testServer(runnerregistry.New()) + + client1, _ := pipeClient(t) + + if ids := s.connectedRunnerIDs(); len(ids) != 0 { + t.Errorf("expected no connected runners before any setClient; got %v", ids) + } + + s.setClient("runner-D", client1) + + ids := s.connectedRunnerIDs() + if len(ids) != 1 || ids[0] != "runner-D" { + t.Errorf("expected [runner-D] after setClient; got %v", ids) + } + + // Current client removed → runner no longer in connected list. + s.removeClient("runner-D", client1) + + if ids := s.connectedRunnerIDs(); len(ids) != 0 { + t.Errorf("expected no connected runners after removeClient; got %v", ids) + } + if got := s.clientFor("runner-D"); got != nil { + t.Errorf("expected nil from clientFor after removeClient; got %v", got) + } +}