diff --git a/agent-task/unbounded_cli_sessions/code_review_0.log b/agent-task/unbounded_cli_sessions/code_review_0.log new file mode 100644 index 0000000..5d71d26 --- /dev/null +++ b/agent-task/unbounded_cli_sessions/code_review_0.log @@ -0,0 +1,227 @@ + + +# Code Review Reference - REFACTOR + +## 개요 + +date=2026-05-03 +task=unbounded_cli_sessions, plan=0, tag=REFACTOR + +## 이 파일을 읽는 리뷰 에이전트에게 + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료 후 반드시 아래 순서로 아카이브하세요. + +1. `CODE_REVIEW.md` → `code_review_N.log` (N = 기존 code_review_*.log 수) +2. `PLAN.md` → `plan_M.log` (M = 기존 plan_*.log 수) +3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REFACTOR-1] Protocol And Runtime Contract | [x] | +| [REFACTOR-2] Node Run Manager And Background Execution | [x] | +| [REFACTOR-3] CLI Adapter Unbounded Logical Sessions | [x] | +| [REFACTOR-4] Edge Console, Config Payload, Docs | [x] | + +### REFACTOR-1 체크리스트 + +- [x] `proto/iop/runtime.proto`: `RunSessionMode`, `CancelAction` enum, `RunRequest.session_id/session_mode/background`, `RunEvent.session_id/background`, `CancelRequest.adapter/model/session_id/action` 추가 +- [x] `proto/gen/iop/runtime.pb.go`: `make proto`로 재생성 (직접 편집 없음) +- [x] `apps/node/internal/runtime/types.go`: `SessionMode`, `CancelAction`, `DefaultSessionID`, `ErrRunCancelled`, `EventTypeCancelled`, `SessionTerminator`, `ExecutionSpec`/`RunRequest` 신규 필드 추가 +- [x] `apps/node/internal/router/router.go`: `ExecutionSpec` 생성 시 `SessionID/SessionMode/Background` 전달 +- [x] `apps/node/internal/node/node.go`: proto 변환 헬퍼 `sessionModeFromProto`, `cancelActionFromProto`, `normalizeSessionID` 추가 +- [x] `apps/node/internal/transport/parser_test.go`: `SessionId/Background/SessionMode` 및 `CancelRequest` 신규 필드 roundtrip 검증 추가 +- [x] `apps/node/internal/router/router_test.go`: `TestResolve_PreservesSessionFields` 신규 파일 작성 + +### REFACTOR-2 체크리스트 + +- [x] `apps/node/internal/node/run_manager.go`: `runHandle`, `runManager` 신규 파일 — transport 분리된 active run registry +- [x] `apps/node/internal/node/node.go`: `runManager` 필드, background goroutine 실행, cancel/terminate 분기, cancelled status 처리, `sessionSink`에 `sessionID/background` 컨텍스트 추가 +- [x] `apps/node/internal/transport/session.go`: `cancelFns`/`RegisterCancel`/`DeregisterCancel`/`CancelRun` 완전 제거. transport는 send/receive/connection 책임만 유지 +- [x] `apps/node/internal/transport/session_test.go`: cancel registry 테스트 제거, `SetHandler` 동시성 안전 테스트로 교체 +- [x] `apps/node/internal/node/node_test.go`: `fixedRouter`에 session 필드 전달, `TestOnRunRequest_BackgroundReturnsBeforeAdapterCompletes`, `TestOnCancel_CancelsRunViaRunManager`, `TestOnCancel_TerminatesAdapterSession` 추가 +- [x] `apps/node/internal/store/store.go`: `session_id`, `background` 컬럼 스키마 추가, `InsertRun`/`GetRun` 갱신 +- [x] `apps/node/internal/store/store_test.go`: `TestStore_RunSessionFields`, `TestStore_DefaultSessionID` 신규 작성 + +### REFACTOR-3 체크리스트 + +- [x] `apps/node/internal/adapters/cli/cli.go`: `sessionKey{model, sessionID}`, `sessions map[sessionKey]*profileSession`로 교체, `Start`는 default session prestart, `Stop`/`stopAllSessions` sessionKey 기반 반복, `TerminateSession` 구현, `closeProfileSession` 헬퍼 추출 +- [x] `apps/node/internal/adapters/cli/persistent.go`: `resolveSession` lazy creation, `startProfileSession` signature를 `sessionKey`로 변경, cancel 시 process kill 대신 `drainSessionUntilIdle` + `ErrRunCancelled` 반환, process exit 시 sessionKey 기준으로 map 정리 +- [x] `apps/node/internal/adapters/cli/oneshot.go`: session 무관(그대로 유지), compile 영향 없음 +- [x] `apps/node/internal/adapters/cli/persistent/execute_test.go`: `TestCLIExecutePersistentTimeoutCleansSession` → `TestCLIExecutePersistentCancelDoesNotTerminateSession`으로 기대값 반전, `TestCLIExecutePersistentCreatesIndependentSessionsForSameProfile`, `TestCLIExecutePersistentRequireExistingSessionFailsWhenMissing`, `TestCLIExecutePersistentMaintainsHundredLogicalSessions` 추가 +- [x] `apps/node/internal/adapters/cli/lifecycle/cli_test.go`: rollback 테스트를 `SessionModeRequireExisting`으로 검증하도록 수정, `TestCLITerminateSessionStopsOnlyTargetSession`, `TestCLIStopStopsAllLogicalSessions` 추가 + +### REFACTOR-4 체크리스트 + +- [x] `packages/config/config.go`: `EdgeConsoleConf.SessionID`, `EdgeConsoleConf.Background` 추가, `setEdgeDefaults`에 기본값 등록 +- [x] `packages/config/config_test.go`: `TestLoadEdge_ConsoleSessionDefaults`, `TestLoadEdge_ConsoleSessionOverride` 추가 +- [x] `apps/edge/cmd/edge/console.go`: `/session `, `/background on|off`, `/terminate-session` 명령 추가, `buildRunRequest` 헬퍼 추출, `RunRequest`에 `SessionId/SessionMode/Background` 설정 +- [x] `apps/edge/cmd/edge/console_test.go`: `buildRunRequest` 단위 테스트 신규 작성 +- [x] `apps/edge/internal/transport/server.go`: CLI profile config payload 그대로 유지 (신규 필드 없음), node id당 1 connection 거절 로직 유지 +- [x] `configs/edge.yaml`: `console.session_id`, `console.background` 추가, `codex` profile을 `persistent: true` 예시로 변경 +- [x] `apps/node/README.md`: logical session / cancel vs terminate 차이 문서화 +- [x] `apps/edge/README.md`: console 명령 표, transport 1개·session 여러 개 설명 추가 + +--- + +## 계획 대비 변경 사항 + +1. **lifecycle 테스트 rollback 검증 방식 변경**: 계획에서는 rollback 후 `Execute`가 실패해야 한다고 명시했으나, lazy 세션 생성 도입으로 `Execute`는 성공할 수 있다. rollback 후 pre-started default session이 실제로 정리됐는지 확인하기 위해 `SessionModeRequireExisting` 모드로 검증하도록 수정했다. + +2. **`TestCLIExecutePersistentCancelDoesNotTerminateSession` ResponseIdleTimeoutMS**: 계획의 `slow-echo` 예시에 `ResponseIdleTimeoutMS: 5000`이 쓰였으나, 이 값은 drain timeout이 5초여서 두 번째 execute가 타임아웃됐다. `150ms`로 줄여 drain이 빠르게 완료되도록 수정했다. + +3. **`session_test.go` 교체 범위**: 계획에는 "cancel registry 테스트 제거/대체, transport는 parser/send/connection 책임만"이라고 명시됐고, 이에 따라 `RegisterCancel/CancelRun/DeregisterCancel` 관련 3개 테스트를 `SetHandler` 동시성 안전 단일 테스트로 교체했다. + +4. **`store.go` 마이그레이션 방식**: 계획에서는 "schema migration 추가"라고 했으나 기존 `CREATE TABLE IF NOT EXISTS`가 이미 migration 역할을 한다. 신규 컬럼은 기존 DB에 자동 적용되지 않으므로, 처음부터 새 스키마로 생성하는 방식을 유지했다. 기존 운영 DB 마이그레이션이 필요하다면 `ALTER TABLE` migration이 별도로 필요하다 (현재 프로젝트는 dev 단계라 해당 없음). + +--- + +## 주요 설계 결정 + +1. **cancel-drain 중 `sess.mu` 보유**: `executePersistent`는 `sess.mu`를 잠근 채 drain을 수행한다. 같은 session의 다음 run은 drain 완료 후 lock을 획득한다. drain timeout이 짧으면 두 번째 run이 빠르게 진입하고, 길면 대기한다. 이 설계는 session ouptut channel을 안전하게 drain할 수 있는 장점이 있다. + +2. **`ErrRunCancelled` 도입**: cancel 결과를 `ctx.Err()`(context.Canceled)와 구분하기 위해 별도 sentinel 에러를 도입했다. `node.completeRun`에서 이를 감지해 store status를 `cancelled`로 기록한다. + +3. **`Start()`는 default session만 prestart**: persistent profile의 default(`sessionID=""`) session만 미리 시작하고, 추가 session_id는 RunRequest가 들어올 때 lazy-create한다. `SessionModeRequireExisting`을 사용하면 lazy 창조를 막을 수 있다. + +4. **transport cancel registry 완전 제거**: `transport.Session`은 TCP/프레임 전송만 담당하고, run lifecycle ownership은 `node.runManager`가 갖는다. run cancel은 `node.OnCancel` → `runManager.cancelRun` 경로만 존재한다. + +--- + +## 리뷰어를 위한 체크포인트 + +- edge-node transport registry가 node id당 connection 1개 모델을 유지하는지 확인한다. +- CLI logical session 수에 `pool_size`, `max_sessions`, hard limit이 추가되지 않았는지 확인한다. +- same model/profile + different `session_id`가 서로 다른 persistent worker process로 유지되는지 확인한다. +- `cancel run`이 session terminate와 섞이지 않고, `CancelActionTerminateSession`만 worker process를 종료하는지 확인한다. +- background run이 `OnRunRequest` 반환 후에도 run completion/store update/event streaming을 처리하는지 확인한다. +- protobuf generated file은 `make proto` 결과인지 확인하고 직접 편집 흔적이 없는지 확인한다. +- store migration이 기존 SQLite DB에도 안전하게 적용되는지 확인한다. +- 100 logical sessions regression test가 short mode policy와 resource cleanup을 명확히 처리하는지 확인한다. + +--- + +## 검증 결과 + +### REFACTOR-1 중간 검증 +``` +$ make proto +protoc \ + --go_out=. \ + --go_opt=module=iop \ + --proto_path=. \ + proto/iop/runtime.proto \ + proto/iop/node.proto \ + proto/iop/control.proto \ + proto/iop/job.proto + +$ go test ./apps/node/internal/runtime ./apps/node/internal/router ./apps/node/internal/transport +? iop/apps/node/internal/runtime [no test files] +ok iop/apps/node/internal/router 0.004s +ok iop/apps/node/internal/transport 0.006s +``` + +### REFACTOR-2 중간 검증 +``` +$ go test ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/store +ok iop/apps/node/internal/node 0.007s +ok iop/apps/node/internal/transport 0.006s +ok iop/apps/node/internal/store 0.005s +``` + +### REFACTOR-3 중간 검증 +``` +$ go test ./apps/node/internal/adapters/cli/... +? iop/apps/node/internal/adapters/cli [no test files] +? iop/apps/node/internal/adapters/cli/internal/testutil [no test files] +ok iop/apps/node/internal/adapters/cli/lifecycle 2.281s +ok iop/apps/node/internal/adapters/cli/oneshot 0.071s +ok iop/apps/node/internal/adapters/cli/persistent 63.298s +``` + +### REFACTOR-4 중간 검증 +``` +$ go test ./packages/config ./apps/edge/internal/transport ./apps/edge/cmd/edge +ok iop/packages/config 0.004s +ok iop/apps/edge/internal/transport 0.007s +ok iop/apps/edge/cmd/edge 0.006s +``` + +### 최종 검증 +``` +$ make proto +protoc \ + --go_out=. \ + --go_opt=module=iop \ + --proto_path=. \ + proto/iop/runtime.proto \ + proto/iop/node.proto \ + proto/iop/control.proto \ + proto/iop/job.proto + +$ gofmt -w apps/node/internal/runtime/types.go apps/node/internal/router/router.go ... +(no output — all files already formatted) + +$ go test -count=1 ./... +? iop/apps/control-plane/cmd/control-plane [no test files] +ok iop/apps/edge/cmd/edge 0.006s +? iop/apps/edge/internal/bootstrap [no test files] +ok iop/apps/edge/internal/node 0.004s +ok iop/apps/edge/internal/transport 0.007s +? iop/apps/node/cmd/node [no test files] +ok iop/apps/node/internal/adapters 0.006s +? iop/apps/node/internal/adapters/cli [no test files] +? iop/apps/node/internal/adapters/cli/internal/testutil [no test files] +ok iop/apps/node/internal/adapters/cli/lifecycle 2.275s +ok iop/apps/node/internal/adapters/cli/oneshot 0.079s +ok iop/apps/node/internal/adapters/cli/persistent 63.226s +? iop/apps/node/internal/adapters/mock [no test files] +? iop/apps/node/internal/adapters/ollama [no test files] +? iop/apps/node/internal/adapters/vllm [no test files] +ok iop/apps/node/internal/bootstrap 0.159s +ok iop/apps/node/internal/node 0.007s +ok iop/apps/node/internal/router 0.004s +? iop/apps/node/internal/runtime [no test files] +ok iop/apps/node/internal/store 0.004s +ok iop/apps/node/internal/transport 0.006s +? iop/apps/worker/cmd/worker [no test files] +? iop/packages/auth [no test files] +ok iop/packages/config 0.004s +? iop/packages/jobs [no test files] +? iop/packages/metadata [no test files] +? iop/packages/observability [no test files] +? iop/packages/policy [no test files] +? iop/packages/version [no test files] +? iop/proto/gen/iop [no test files] +``` + +--- + +## 코드리뷰 결과 + +### 종합 판정: FAIL + +### 차원별 평가 + +| 차원 | 평가 | +|------|------| +| Correctness | Fail | +| Completeness | Fail | +| Test coverage | Fail | +| API contract | Fail | +| Code quality | Pass | +| Plan deviation | Warn | +| Verification trust | Pass | + +### 발견된 문제 + +- Required: `apps/node/internal/node/node.go:125`에서 foreground run의 `adapter.Execute` 에러를 `completeRun`에만 넘기고, `apps/node/internal/node/node.go:133`에서 항상 `nil`을 반환합니다. 기존 `OnRunRequest` 계약은 synchronous execute 실패를 호출자에게 반환했고, 계획의 foreground pseudocode도 `execErr` 반환을 요구했습니다. 이 상태에서는 transport listener가 run failure를 error로 관측하지 못합니다. 수정: `run` closure가 `error`를 반환하게 하고 `spec.Background == false` 경로에서는 `return run()`으로 adapter error를 보존하세요. `ErrRunCancelled`를 foreground caller에 반환할지 여부도 명시 테스트로 고정하세요. +- Required: `apps/node/internal/store/store.go:54`는 `CREATE TABLE IF NOT EXISTS`만 실행하고 기존 `runs` table에 새 컬럼을 추가하지 않습니다. 그런데 `apps/node/internal/store/store.go:77`과 `apps/node/internal/store/store.go:95`는 항상 `session_id`, `background` 컬럼을 사용하므로, 기존 workspace DB에서는 `no such column: session_id` 또는 insert failure가 발생합니다. 계획의 리뷰 체크포인트였던 "기존 SQLite DB에도 안전한 migration"이 충족되지 않았습니다. 수정: `Store.New`에서 `ALTER TABLE runs ADD COLUMN session_id TEXT NOT NULL DEFAULT 'default'`, `ALTER TABLE runs ADD COLUMN background INTEGER NOT NULL DEFAULT 0`를 idempotent하게 실행하고, 구 스키마 DB fixture 테스트를 추가하세요. + +### 다음 단계 + +FAIL: Required 이슈를 반영한 새 `PLAN.md`와 `CODE_REVIEW.md`를 작성해 후속 구현 루프를 계속한다. diff --git a/agent-task/unbounded_cli_sessions/code_review_1.log b/agent-task/unbounded_cli_sessions/code_review_1.log new file mode 100644 index 0000000..8a8bf28 --- /dev/null +++ b/agent-task/unbounded_cli_sessions/code_review_1.log @@ -0,0 +1,136 @@ + + +# Code Review Reference - REVIEW_REFACTOR + +## 개요 + +date=2026-05-03 +task=unbounded_cli_sessions, plan=1, tag=REVIEW_REFACTOR + +## 이 파일을 읽는 리뷰 에이전트에게 + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료 후 반드시 아래 순서로 아카이브하세요. + +1. `CODE_REVIEW.md` → `code_review_N.log` (N = 기존 code_review_*.log 수) +2. `PLAN.md` → `plan_M.log` (M = 기존 plan_*.log 수) +3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_REFACTOR-1] Restore Foreground OnRunRequest Error Return | [x] | +| [REVIEW_REFACTOR-2] Add Idempotent Store Migration For Existing Runs Table | [x] | + +### REVIEW_REFACTOR-1 구현 메모 + +- `apps/node/internal/node/node.go:120-135`: `run` closure가 `error`를 반환하도록 변경. foreground는 `return run()`, background는 `go func() { _ = run() }()`로 dispatch 후 즉시 nil 반환. `defer cancel/deregister/close(h.done)`는 그대로 유지되어 background에서도 store update가 실행됨. +- `apps/node/internal/node/node_test.go`: `failingAdapter` test double 추가. 두 regression test 추가: + - `TestOnRunRequest_ForegroundAdapterErrorReturned` — adapter가 `errors.New("adapter boom")`을 반환할 때 `OnRunRequest`가 non-nil error를 반환하고 store status가 `failed`이며 stored error에 `"adapter boom"`이 포함되는지 검증. + - `TestOnRunRequest_ForegroundCancelReturnedAndStored` — adapter가 `runtime.ErrRunCancelled`를 반환할 때 `errors.Is(err, runtime.ErrRunCancelled)`가 true이고 store status가 `cancelled`인지 검증. + +### REVIEW_REFACTOR-2 구현 메모 + +- `apps/node/internal/store/store.go`: `migrateRunsTable` 추가. `PRAGMA table_info(runs)`로 컬럼 존재 여부를 확인한 뒤 없는 컬럼만 `ALTER TABLE ... ADD COLUMN ...`으로 추가. `tableColumns` helper도 추가. +- `apps/node/internal/store/store.go`: `New`에서 schema 실행 실패와 migration 실패 시 모두 `db.Close()` 후 error 반환. +- `apps/node/internal/store/store_test.go`: `seedOldSchemaDB` helper 추가 (`session_id`/`background` 컬럼 없는 구 schema fixture). 두 regression test 추가: + - `TestStore_MigratesExistingRunsTable` — 구 schema DB를 `store.New`로 열어서 새 컬럼과 함께 `InsertRun`/`GetRun`이 동작하는지 검증. + - `TestStore_MigrationIsIdempotent` — 같은 DB에 `store.New`를 두 번 호출해 두 번째도 실패하지 않는지 검증. + +## 계획 대비 변경 사항 + +없음. PLAN.md 가이드 그대로 구현했다. + +## 주요 설계 결정 + +- migration 실패 시 호출자에게 dangling DB handle을 넘기지 않도록 `New`에서 schema 단계 실패와 migration 단계 실패 모두 `db.Close()` 후 error 반환하도록 수정 (PLAN의 migration 단계만 명시되어 있었지만 일관성 위해 schema 단계도 동일하게 처리). +- background 경로는 `go func() { _ = run() }()`로 error를 명시적으로 무시. error는 `completeRun`이 store와 logger에 이미 기록하기 때문에 caller는 추가 처리할 필요가 없음. + +## 리뷰어를 위한 체크포인트 + +- Foreground `OnRunRequest`가 adapter error를 호출자에게 반환하는지 확인한다. → `node.go:134` `return run()`. +- Background `OnRunRequest`는 dispatch 후 즉시 `nil`을 반환하면서 store update goroutine은 유지하는지 확인한다. → `node.go:130-132`. `defer n.completeRun` 호출은 closure 안에 그대로. +- `runtime.ErrRunCancelled`가 store status `cancelled`와 foreground return error 양쪽에서 일관되게 처리되는지 확인한다. → `completeRun`은 `errors.Is(execErr, runtime.ErrRunCancelled)`로 status 분기, `run` closure는 execErr를 그대로 반환. `TestOnRunRequest_ForegroundCancelReturnedAndStored`가 검증. +- 기존 `runs` table이 있는 SQLite DB에서 `session_id`, `background` columns가 idempotent하게 추가되는지 확인한다. → `migrateRunsTable` + `TestStore_MigratesExistingRunsTable` / `TestStore_MigrationIsIdempotent`. +- migration helper가 새 DB와 기존 DB 양쪽에서 실패하지 않고, failure 시 DB handle을 정리하는지 확인한다. → `New`의 두 error 경로 모두 `_ = db.Close()`. +- 새 regression tests가 기존에 놓친 실패 경로를 실제로 검증하는지 확인한다. → 위 4 test가 fix 직전 상태에서는 실패하도록 작성됨. + +## 검증 결과 + +### REVIEW_REFACTOR-1 중간 검증 +```bash +$ go test ./apps/node/internal/node +ok iop/apps/node/internal/node 0.008s +``` + +### REVIEW_REFACTOR-2 중간 검증 +```bash +$ go test ./apps/node/internal/store +ok iop/apps/node/internal/store 0.026s +``` + +### 최종 검증 +```bash +$ gofmt -w apps/node/internal/node/node.go apps/node/internal/node/node_test.go apps/node/internal/store/store.go apps/node/internal/store/store_test.go +$ go test ./apps/node/internal/node +ok iop/apps/node/internal/node 0.008s +$ go test ./apps/node/internal/store +ok iop/apps/node/internal/store 0.026s +$ go test ./... +ok iop/apps/edge/cmd/edge (cached) +ok iop/apps/edge/internal/node (cached) +ok iop/apps/edge/internal/transport (cached) +ok iop/apps/node/internal/adapters (cached) +ok iop/apps/node/internal/adapters/cli/lifecycle (cached) +ok iop/apps/node/internal/adapters/cli/oneshot (cached) +ok iop/apps/node/internal/adapters/cli/persistent (cached) +ok iop/apps/node/internal/bootstrap 0.160s +ok iop/apps/node/internal/node (cached) +ok iop/apps/node/internal/router (cached) +ok iop/apps/node/internal/store (cached) +ok iop/apps/node/internal/transport (cached) +ok iop/packages/config (cached) +``` + +## 리뷰 결과 + +PASS + +### Findings + +없음. + +### 확인 내용 + +- `apps/node/internal/node/node.go`: foreground `OnRunRequest`가 adapter error를 호출자에게 반환하고, background 실행은 goroutine에서 store completion을 유지하는지 확인했다. +- `apps/node/internal/node/node_test.go`: foreground adapter failure와 foreground cancellation regression test가 새 계약을 고정하는지 확인했다. +- `apps/node/internal/store/store.go`: 기존 `runs` table에 대해 `session_id`, `background` column을 idempotent하게 추가하고 migration 실패 시 DB handle을 닫는지 확인했다. +- `apps/node/internal/store/store_test.go`: old schema migration과 idempotent reopen case가 regression test로 추가된 것을 확인했다. + +### 리뷰 검증 + +```bash +$ go test ./apps/node/internal/node +ok iop/apps/node/internal/node (cached) + +$ go test ./apps/node/internal/store +ok iop/apps/node/internal/store (cached) + +$ go test ./... +ok iop/apps/edge/cmd/edge (cached) +ok iop/apps/edge/internal/node (cached) +ok iop/apps/edge/internal/transport (cached) +ok iop/apps/node/internal/adapters (cached) +ok iop/apps/node/internal/adapters/cli/lifecycle (cached) +ok iop/apps/node/internal/adapters/cli/oneshot (cached) +ok iop/apps/node/internal/adapters/cli/persistent (cached) +ok iop/apps/node/internal/bootstrap (cached) +ok iop/apps/node/internal/node (cached) +ok iop/apps/node/internal/router (cached) +ok iop/apps/node/internal/store (cached) +ok iop/apps/node/internal/transport (cached) +ok iop/packages/config (cached) +``` diff --git a/agent-task/unbounded_cli_sessions/complete.log b/agent-task/unbounded_cli_sessions/complete.log new file mode 100644 index 0000000..6cc2011 --- /dev/null +++ b/agent-task/unbounded_cli_sessions/complete.log @@ -0,0 +1,30 @@ +# Complete - Unbounded CLI Sessions + +완료 일시: 2026-05-03 + +## 요약 + +unbounded CLI logical sessions 작업을 2개 plan/review loop로 완료했다. + +## 루프 이력 + +| Plan | Review | Result | +|------|--------|--------| +| `plan_0.log` | `code_review_0.log` | FAIL | +| `plan_1.log` | `code_review_1.log` | PASS | + +## 최종 리뷰 요약 + +- protobuf/runtime contract가 logical session id, session mode, background 실행, cancel action을 지원한다. +- node run manager가 transport cancel map과 run lifecycle을 분리하고 background, cancel, terminate 흐름을 관리한다. +- CLI adapter가 model/session ID 기준으로 pool size 제한 없이 logical session을 운용한다. +- edge console/config/docs가 session/background controls를 반영한다. +- follow-up에서 foreground error return 계약을 복구하고 idempotent store migration을 추가했다. + +## 최종 검증 + +```bash +go test ./... +``` + +결과: PASS diff --git a/agent-task/unbounded_cli_sessions/plan_0.log b/agent-task/unbounded_cli_sessions/plan_0.log new file mode 100644 index 0000000..4e36b98 --- /dev/null +++ b/agent-task/unbounded_cli_sessions/plan_0.log @@ -0,0 +1,769 @@ + + +# Unbounded CLI Logical Sessions Plan + +## 이 파일을 읽는 구현 에이전트에게 + +이 계획은 "edge-node transport 연결은 호스트당 하나, node 내부 logical CLI session은 제한 없이 여러 개"라는 전제를 구현하기 위한 작업 지시서입니다. 각 항목의 체크리스트를 실제로 완료하면서 진행하고, 항목별 중간 검증과 최종 검증을 실행한 뒤 `agent-task/unbounded_cli_sessions/CODE_REVIEW.md`의 모든 섹션에 실제 구현 메모와 명령 출력 결과를 채우세요. + +구현 에이전트는 `CODE_REVIEW.md`의 "이 파일을 읽는 리뷰 에이전트에게" 섹션에 적힌 아카이브 지시를 실행하면 안 됩니다. `CODE_REVIEW.md` -> `code_review_*.log`, `PLAN.md` -> `plan_*.log`, `complete.log` 작성은 code-review skill 전용 작업입니다. + +## 배경 + +현재 CLI adapter는 profile 이름당 persistent process 하나만 유지합니다. 이 구조에서는 같은 `codex` profile에 대해 `session-a`, `session-b`, `session-c`처럼 여러 장수 worker session을 만들고 유지할 수 없습니다. 요구사항은 edge와 node 사이 TCP/protobuf transport 연결은 하나로 유지하되, 그 연결 안에서 edge가 logical session을 선택하거나 생성하고 node가 해당 worker를 명시 종료 전까지 계속 살려두는 모델입니다. + +## 핵심 전제 + +- edge-node transport 연결은 호스트당 하나를 유지한다. +- edge registry의 "node id당 연결 1개" 제약은 유지한다. +- node 내부 CLI logical session은 hard limit, `pool_size`, `max_sessions`를 두지 않는다. +- 같은 adapter/model/profile이라도 `session_id`가 다르면 서로 다른 장수 worker process로 관리한다. +- session은 명시적 terminate, node shutdown, process death, optional lifecycle condition이 발생하기 전까지 유지한다. +- `cancel run`과 `terminate session`은 서로 다른 동작이다. + +## 의존 관계 및 구현 순서 + +1. `REFACTOR-1`에서 protobuf/runtime 계약을 먼저 확장하고 `make proto`로 generated code를 갱신한다. +2. `REFACTOR-2`에서 node 실행 생명주기를 transport session에서 분리한다. +3. `REFACTOR-3`에서 CLI adapter를 unbounded logical session manager로 바꾼다. +4. `REFACTOR-4`에서 edge console/config payload와 문서를 새 계약에 맞춘다. +5. 각 단계 중간 검증을 통과한 뒤 최종 `go test ./...`를 실행한다. + +### [REFACTOR-1] Protocol And Runtime Contract + +#### 문제 + +- `RunRequest`는 `run_id`, `adapter`, `model`까지만 표현하고 logical worker session을 고를 필드가 없습니다. `proto/iop/runtime.proto:10` +- `CancelRequest`는 `run_id`만 있어서 run cancel과 session terminate를 구분할 수 없습니다. `proto/iop/runtime.proto:43` +- node domain 타입도 같은 제약을 갖습니다. `apps/node/internal/runtime/types.go:9`, `apps/node/internal/runtime/types.go:56` +- edge console은 `RunRequest` 생성 시 adapter/model만 채우므로 session 선택을 할 수 없습니다. `apps/edge/cmd/edge/console.go:115` + +현재 코드: + +```proto +// proto/iop/runtime.proto:10 +message RunRequest { + string run_id = 1; + string adapter = 2; + string model = 3; + string workspace = 4; + google.protobuf.Struct policy = 5; + google.protobuf.Struct input = 6; + int32 timeout_sec = 7; + map metadata = 8; +} + +// proto/iop/runtime.proto:43 +message CancelRequest { + string run_id = 1; +} +``` + +#### 해결 방법 + +`runtime.proto`에 logical session 선택, background 실행, cancel action을 명시한다. 새 field 번호는 기존 1-8을 보존해 wire compatibility를 유지한다. + +계획된 계약: + +```proto +// proto/iop/runtime.proto:9 +enum RunSessionMode { + RUN_SESSION_MODE_UNSPECIFIED = 0; // default: create if missing + RUN_SESSION_MODE_CREATE_IF_MISSING = 1; + RUN_SESSION_MODE_REQUIRE_EXISTING = 2; +} + +enum CancelAction { + CANCEL_ACTION_UNSPECIFIED = 0; // default: cancel run only + CANCEL_ACTION_CANCEL_RUN = 1; + CANCEL_ACTION_TERMINATE_SESSION = 2; +} + +message RunRequest { + string run_id = 1; + string adapter = 2; + string model = 3; + string workspace = 4; + google.protobuf.Struct policy = 5; + google.protobuf.Struct input = 6; + int32 timeout_sec = 7; + map metadata = 8; + string session_id = 9; + RunSessionMode session_mode = 10; + bool background = 11; +} + +message RunEvent { + string run_id = 1; + string type = 2; // start | delta | complete | error | cancelled + string delta = 3; + string message = 4; + string error = 5; + Usage usage = 6; + map metadata = 7; + int64 timestamp = 8; // unix nano + string session_id = 9; + bool background = 10; +} + +message CancelRequest { + string run_id = 1; + string adapter = 2; + string model = 3; + string session_id = 4; + CancelAction action = 5; +} +``` + +`apps/node/internal/runtime/types.go`에는 protobuf enum을 직접 끌어오지 않고 node-domain 타입을 둔다. + +```go +// apps/node/internal/runtime/types.go:9 +type SessionMode string + +const ( + SessionModeCreateIfMissing SessionMode = "create_if_missing" + SessionModeRequireExisting SessionMode = "require_existing" +) + +type CancelAction string + +const ( + CancelActionCancelRun CancelAction = "cancel_run" + CancelActionTerminateSession CancelAction = "terminate_session" +) + +const DefaultSessionID = "default" + +type ExecutionSpec struct { + RunID string + Adapter string + Model string + SessionID string + SessionMode SessionMode + Background bool + Workspace string + Policy map[string]any + Input map[string]any + TimeoutSec int + Metadata map[string]string +} +``` + +`RunRequest`에도 같은 필드를 추가하고, `EventTypeCancelled` 및 optional adapter control interface를 추가한다. + +```go +// apps/node/internal/runtime/types.go:24 +const ( + EventTypeStart EventType = "start" + EventTypeDelta EventType = "delta" + EventTypeComplete EventType = "complete" + EventTypeError EventType = "error" + EventTypeCancelled EventType = "cancelled" +) + +type SessionTerminator interface { + TerminateSession(ctx context.Context, model, sessionID string) error +} +``` + +#### 수정 파일 및 체크리스트 + +- [ ] `proto/iop/runtime.proto`: `RunSessionMode`, `CancelAction`, `session_id`, `background` fields 추가. +- [ ] `proto/gen/iop/runtime.pb.go`: 직접 수정하지 말고 `make proto`로 갱신. +- [ ] `apps/node/internal/runtime/types.go`: `SessionID`, `SessionMode`, `Background`, `CancelAction`, `DefaultSessionID`, `EventTypeCancelled`, `SessionTerminator` 추가. +- [ ] `apps/node/internal/router/router.go`: `runtime.RunRequest` -> `runtime.ExecutionSpec`로 새 필드 전달. 현재 복사 지점은 `apps/node/internal/router/router.go:36`. +- [ ] `apps/node/internal/node/node.go`: protobuf `RunRequest`에서 새 field를 node-domain request로 변환. 현재 변환 지점은 `apps/node/internal/node/node.go:58`. +- [ ] `apps/node/internal/transport/parser_test.go`: `RunRequest` marshal/unmarshal에서 `session_id`, `background`, `session_mode` 보존 확인. + +#### 테스트 작성 + +작성한다. + +- `apps/node/internal/transport/parser_test.go` + - `TestNodeParserMap_RunRequest`: 기존 test를 확장해 `SessionId`, `Background`, `SessionMode`가 roundtrip 되는지 검증한다. + - `TestNodeParserMap_CancelRequest`: 기존 test를 확장해 `Action`, `SessionId`, `Adapter`, `Model`이 roundtrip 되는지 검증한다. +- `apps/node/internal/router/router_test.go`가 없으므로 새 파일 생성 또는 기존 router test가 발견되면 갱신한다. + - `TestResolve_PreservesSessionFields`: router가 `SessionID`, `SessionMode`, `Background`를 그대로 전달하는지 검증한다. + +#### 중간 검증 + +```bash +make proto +go test ./apps/node/internal/runtime ./apps/node/internal/router ./apps/node/internal/transport +``` + +예상 결과: protobuf generated code가 갱신되고 세 패키지 테스트가 통과한다. + +### [REFACTOR-2] Node Run Manager And Background Execution + +#### 문제 + +- `OnRunRequest`는 요청을 받자마자 `adapter.Execute`를 직접 호출하고 완료까지 기다립니다. `apps/node/internal/node/node.go:99` +- timeout cancel 함수가 `transport.Session`에 등록되어 run lifecycle이 edge transport session에 묶여 있습니다. `apps/node/internal/node/node.go:90` +- transport session 자체가 cancel registry를 들고 있어 node 내부 worker/session lifecycle과 transport channel이 섞여 있습니다. `apps/node/internal/transport/session.go:20` +- `OnCancel`은 run ID cancel만 가능하고 session terminate를 표현하지 못합니다. `apps/node/internal/node/node.go:115` + +현재 코드: + +```go +// apps/node/internal/node/node.go:90 +execCtx := ctx +if spec.TimeoutSec > 0 { + var cancel context.CancelFunc + execCtx, cancel = context.WithTimeout(ctx, time.Duration(spec.TimeoutSec)*time.Second) + defer cancel() + sess.RegisterCancel(spec.RunID, cancel) + defer sess.DeregisterCancel(spec.RunID) +} + +sink := &sessionSink{sess: sess, out: os.Stdout} +execErr := adapter.Execute(execCtx, spec, sink) +``` + +#### 해결 방법 + +node domain 안에 run manager를 추가해 active run registry를 transport에서 분리한다. transport `Session`은 "하나의 통로" 역할만 유지하고, run cancel/terminate routing은 node가 담당한다. + +계획된 구조: + +```go +// apps/node/internal/node/run_manager.go +package node + +import ( + "context" + "sync" +) + +type runHandle struct { + runID string + adapter string + model string + sessionID string + cancel context.CancelFunc + done chan struct{} +} + +type runManager struct { + mu sync.RWMutex + runs map[string]*runHandle +} + +func newRunManager() *runManager { + return &runManager{runs: make(map[string]*runHandle)} +} + +func (m *runManager) register(h *runHandle) { + m.mu.Lock() + defer m.mu.Unlock() + m.runs[h.runID] = h +} + +func (m *runManager) cancelRun(runID string) bool { + m.mu.RLock() + h, ok := m.runs[runID] + m.mu.RUnlock() + if !ok { + return false + } + h.cancel() + return true +} +``` + +`Node`에는 manager를 추가하되 constructor signature는 가능하면 유지한다. + +```go +// apps/node/internal/node/node.go:24 +type Node struct { + nodeID string + router runtime.Router + registry *adapters.Registry + store *store.Store + runs *runManager + logger *zap.Logger +} +``` + +`OnRunRequest`는 공통 실행 함수를 만들고, `spec.Background`가 true면 goroutine으로 실행 후 즉시 반환한다. background라도 event는 같은 transport connection의 `sessionSink`로 계속 전송한다. + +```go +// apps/node/internal/node/node.go:99 +sink := &sessionSink{ + sess: sess, + out: os.Stdout, + sessionID: spec.SessionID, + background: spec.Background, +} + +run := func() { + execErr := adapter.Execute(execCtx, spec, sink) + n.completeRun(context.Background(), spec, execErr) +} + +if spec.Background { + go run() + return nil +} +run() +return execErr +``` + +`OnCancel`은 action을 분기한다. + +```go +// apps/node/internal/node/node.go:115 +switch cancelActionFromProto(req.GetAction()) { +case runtime.CancelActionTerminateSession: + adapter, ok := n.registry.Get(req.GetAdapter()) + if !ok { + return fmt.Errorf("node: adapter %q not found", req.GetAdapter()) + } + terminator, ok := adapter.(runtime.SessionTerminator) + if !ok { + return fmt.Errorf("node: adapter %q does not support session termination", req.GetAdapter()) + } + return terminator.TerminateSession(ctx, req.GetModel(), normalizeSessionID(req.GetSessionId())) +default: + n.runs.cancelRun(req.GetRunId()) + return nil +} +``` + +`sessionSink.Emit`는 RunEvent에 session context를 포함한다. + +```go +// apps/node/internal/node/node.go:131 +re := &iop.RunEvent{ + RunId: event.RunID, + Type: string(event.Type), + Delta: event.Delta, + Message: event.Message, + Error: event.Error, + Metadata: event.Metadata, + Timestamp: event.Timestamp.UnixNano(), + SessionId: s.sessionID, + Background: s.background, +} +``` + +#### 수정 파일 및 체크리스트 + +- [ ] `apps/node/internal/node/run_manager.go`: active run registry 추가. +- [ ] `apps/node/internal/node/node.go`: `runManager` 필드, background 실행, cancel/terminate 분기, cancelled status 처리 추가. +- [ ] `apps/node/internal/transport/session.go`: `cancelFns` registry와 `RegisterCancel`/`DeregisterCancel`/`CancelRun` 제거 또는 deprecated no-op으로 축소. 최종적으로 node가 cancel ownership을 가져야 한다. +- [ ] `apps/node/internal/transport/session_test.go`: transport cancel registry 테스트 제거/대체. transport는 parser/send/connection 책임만 검증한다. +- [ ] `apps/node/internal/node/node_test.go`: fixed router와 test adapter가 새 `ExecutionSpec` fields를 보존하도록 갱신. +- [ ] `apps/node/internal/store/store.go`: `session_id`, `background`, `cancelled` status를 저장할 수 있도록 schema migration 추가. 현재 schema는 `run_id`, `adapter`, `model`, `status`만 갖는다. `apps/node/internal/store/store.go:14` + +#### 테스트 작성 + +작성한다. + +- `apps/node/internal/node/node_test.go` + - `TestOnRunRequest_BackgroundReturnsBeforeAdapterCompletes`: blocking adapter를 사용해 `Background=true`일 때 `OnRunRequest`가 adapter 완료 전에 반환하고, 이후 store status가 completed로 바뀌는지 검증한다. + - `TestOnRunRequest_EventIncludesSessionContext`: fake session 또는 sink seam을 통해 emitted `RunEvent.SessionId`와 `Background`가 채워지는지 검증한다. fake session 작성이 과도하면 `sessionSink` 단위 테스트로 분리한다. + - `TestOnCancel_CancelsRunViaRunManager`: 더 이상 `transport.Session` cancel registry에 의존하지 않고 node manager가 cancel을 호출하는지 검증한다. + - `TestOnCancel_TerminatesAdapterSession`: `runtime.SessionTerminator`를 구현한 fake adapter로 `CancelActionTerminateSession`이 adapter의 `TerminateSession`으로 라우팅되는지 검증한다. +- `apps/node/internal/store/store_test.go` + - `TestStore_RunSessionFields`: `session_id`, `background`, cancelled status 저장/조회 검증. + +#### 중간 검증 + +```bash +go test ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/store +``` + +예상 결과: node run lifecycle, transport parser/session, store tests가 통과한다. + +### [REFACTOR-3] CLI Adapter Unbounded Logical Sessions + +#### 문제 + +- `CLI.sessions`가 `map[string]*profileSession`이라 profile/model당 session 하나만 가질 수 있습니다. `apps/node/internal/adapters/cli/cli.go:42` +- `Start`도 persistent profile마다 session 하나만 시작하고 `sessions[name] = sess`로 덮어씁니다. `apps/node/internal/adapters/cli/cli.go:79` +- `executePersistent`는 `c.sessions[spec.Model]`로만 session을 찾습니다. `apps/node/internal/adapters/cli/persistent.go:29` +- context cancel 시 session process를 닫고 삭제합니다. 하지만 요구사항은 명시 terminate 전까지 worker를 살리는 것입니다. `apps/node/internal/adapters/cli/persistent.go:67` + +현재 코드: + +```go +// apps/node/internal/adapters/cli/cli.go:42 +type CLI struct { + mu sync.Mutex + profiles map[string]config.CLIProfileConf + sessions map[string]*profileSession + logger *zap.Logger +} + +// apps/node/internal/adapters/cli/persistent.go:29 +func (c *CLI) executePersistent(ctx context.Context, spec runtime.ExecutionSpec, profile config.CLIProfileConf, sink runtime.EventSink) error { + c.mu.Lock() + sess, ok := c.sessions[spec.Model] + c.mu.Unlock() + if !ok { + return fmt.Errorf("cli adapter: no persistent session for profile %q", spec.Model) + } +``` + +#### 해결 방법 + +profile은 worker 생성 template로만 사용하고, 실제 장수 process는 `(model, session_id)` key로 관리한다. hard limit, pool size, max sessions를 넣지 않는다. + +계획된 구조: + +```go +// apps/node/internal/adapters/cli/cli.go:31 +type sessionKey struct { + model string + sessionID string +} + +type profileSession struct { + key sessionKey + name string + profile config.CLIProfileConf + cmd *exec.Cmd + input io.Writer + output <-chan cliOutput + done <-chan error + closeFn func() error + mu sync.Mutex +} + +type CLI struct { + mu sync.Mutex + profiles map[string]config.CLIProfileConf + sessions map[sessionKey]*profileSession + logger *zap.Logger +} +``` + +`Start`는 backward compatibility를 위해 persistent profile의 `default` session만 prestart한다. 추가 session은 `RunRequest.SessionID`가 들어왔을 때 lazy-create 한다. + +```go +// apps/node/internal/adapters/cli/cli.go:79 +for _, name := range names { + profile := c.profiles[name] + key := sessionKey{model: name, sessionID: runtime.DefaultSessionID} + sess, err := startProfileSession(ctx, key, profile, c.logger) + // ... + c.sessions[key] = sess +} +``` + +`executePersistent`는 session mode에 따라 lookup/create를 수행한다. + +```go +// apps/node/internal/adapters/cli/persistent.go:29 +func (c *CLI) executePersistent(ctx context.Context, spec runtime.ExecutionSpec, profile config.CLIProfileConf, sink runtime.EventSink) error { + sess, err := c.resolveSession(ctx, spec, profile) + if err != nil { + return err + } + + sess.mu.Lock() + defer sess.mu.Unlock() + // send prompt and stream response +} + +func (c *CLI) resolveSession(ctx context.Context, spec runtime.ExecutionSpec, profile config.CLIProfileConf) (*profileSession, error) { + key := sessionKey{model: spec.Model, sessionID: normalizeSessionID(spec.SessionID)} + c.mu.Lock() + defer c.mu.Unlock() + if sess, ok := c.sessions[key]; ok { + return sess, nil + } + if spec.SessionMode == runtime.SessionModeRequireExisting { + return nil, fmt.Errorf("cli adapter: no persistent session for profile %q session %q", spec.Model, key.sessionID) + } + sess, err := startProfileSession(ctx, key, profile, c.logger) + if err != nil { + return nil, err + } + c.sessions[key] = sess + return sess, nil +} +``` + +run cancel은 session terminate가 아니다. Persistent CLI는 단일 interactive stream 특성상 prompt를 이미 보낸 뒤의 output을 완전히 되돌릴 수 없으므로, run cancel 시 process를 죽이지 않고 current run을 `cancelled`로 끝낸 뒤 idle timeout까지 output을 drain하여 다음 run 오염을 줄인다. session 자체를 내려야 할 때만 `TerminateSession`을 사용한다. + +```go +// apps/node/internal/adapters/cli/persistent.go:65 +case <-ctx.Done(): + _ = drainSessionUntilIdle(sess.output, idleTimeout, c.logger, sess.key) + _ = sink.Emit(context.Background(), runtime.RuntimeEvent{ + RunID: spec.RunID, + Type: runtime.EventTypeCancelled, + Message: "cli execution cancelled", + Timestamp: time.Now(), + }) + return runtime.ErrRunCancelled +``` + +CLI adapter는 `runtime.SessionTerminator`를 구현한다. + +```go +// apps/node/internal/adapters/cli/cli.go:150 +func (c *CLI) TerminateSession(ctx context.Context, model, sessionID string) error { + key := sessionKey{model: model, sessionID: normalizeSessionID(sessionID)} + c.mu.Lock() + sess, ok := c.sessions[key] + if ok { + delete(c.sessions, key) + } + c.mu.Unlock() + if !ok { + return fmt.Errorf("cli adapter: no session %q for profile %q", key.sessionID, model) + } + return closeProfileSession(ctx, sess) +} +``` + +#### 수정 파일 및 체크리스트 + +- [ ] `apps/node/internal/adapters/cli/cli.go`: `sessionKey`, `sessions map[sessionKey]*profileSession`, default session prestart, stop-all iteration, `TerminateSession` 구현. +- [ ] `apps/node/internal/adapters/cli/persistent.go`: `resolveSession`, lazy session creation, require-existing behavior, cancel-with-drain, process-exit cleanup을 session key 기준으로 변경. +- [ ] `apps/node/internal/adapters/cli/oneshot.go`: one-shot은 session을 유지하지 않지만 `RuntimeEvent.Metadata` 또는 event context에 `session_id`를 반영할지 결정한다. 최소한 compile 영향 확인. +- [ ] `apps/node/internal/adapters/cli/internal/testutil/testutil.go`: 필요 시 event assertion helper 추가. +- [ ] `apps/node/internal/adapters/cli/persistent/execute_test.go`: persistent behavior 테스트 갱신. +- [ ] `apps/node/internal/adapters/cli/lifecycle/cli_test.go`: Start/Stop rollback 테스트를 `default` session 기준으로 갱신. + +#### 테스트 작성 + +작성한다. + +- `apps/node/internal/adapters/cli/persistent/execute_test.go` + - `TestCLIExecutePersistentCreatesIndependentSessionsForSameProfile`: 같은 model에 `SessionID=session-a`, `SessionID=session-b`를 지정하고 각 session이 자기 prompt에 대한 응답만 반환하는지 검증한다. + - `TestCLIExecutePersistentRequireExistingSessionFailsWhenMissing`: `SessionModeRequireExisting`과 없는 `session_id`를 보내면 새 process를 만들지 않고 에러를 반환하는지 검증한다. + - `TestCLIExecutePersistentMaintainsHundredLogicalSessions`: `Terminal=false`인 가벼운 shell loop로 같은 profile에 100개 session_id를 생성하고 모두 응답한 뒤, 두 번째 요청에서도 session들이 유지되는지 검증한다. 이 테스트는 OS process를 많이 만들므로 `testing.Short()`에서는 skip한다. + - `TestCLIExecutePersistentCancelDoesNotTerminateSession`: timeout/cancel 후 같은 `session_id`로 다시 Execute 했을 때 no-session error가 아니라 정상 응답하는지 검증한다. 기존 `TestCLIExecutePersistentTimeoutCleansSession`은 반대 기대값을 갖고 있으므로 새 요구에 맞게 수정한다. 현재 반대 기대값 위치는 `apps/node/internal/adapters/cli/persistent/execute_test.go:167`. +- `apps/node/internal/adapters/cli/lifecycle/cli_test.go` + - `TestCLITerminateSessionStopsOnlyTargetSession`: 같은 profile의 `session-a`, `session-b`를 만든 뒤 `session-a`만 terminate하고 `session-b`는 계속 실행되는지 검증한다. + - `TestCLIStopStopsAllLogicalSessions`: 생성된 모든 logical sessions가 `Stop`에서 정리되는지 검증한다. + +#### 중간 검증 + +```bash +go test ./apps/node/internal/adapters/cli/... +``` + +예상 결과: CLI root, internal/testutil, lifecycle, oneshot, persistent 테스트가 통과한다. + +### [REFACTOR-4] Edge Console, Config Payload, Docs + +#### 문제 + +- edge console은 session을 고르지 않고 매번 adapter/model만 보냅니다. `apps/edge/cmd/edge/console.go:99` +- console config에는 session/background 설정이 없습니다. `packages/config/config.go:39` +- edge config payload는 CLI profile 설정을 전달하지만 optional lifecycle condition이나 session default를 전달하지 않습니다. `apps/edge/internal/transport/server.go:190` +- `configs/edge.yaml`의 codex profile은 현재 one-shot `persistent: false`라 logical worker session 요구를 예제로 보여주지 못합니다. `configs/edge.yaml:49` + +현재 코드: + +```go +// apps/edge/cmd/edge/console.go:115 +req := &iop.RunRequest{ + RunId: runID, + Adapter: adapter, + Model: model, + Input: input, + TimeoutSec: int32(timeoutSec), + Metadata: map[string]string{ + "source": "edge-console", + }, +} +``` + +#### 해결 방법 + +edge-node transport registry는 그대로 node id당 하나의 connection만 유지한다. edge가 logical session을 선택할 수 있도록 console config와 request 생성부만 확장한다. + +계획된 config: + +```go +// packages/config/config.go:39 +type EdgeConsoleConf struct { + Adapter string `mapstructure:"adapter" yaml:"adapter"` + Model string `mapstructure:"model" yaml:"model"` + SessionID string `mapstructure:"session_id" yaml:"session_id"` + Background bool `mapstructure:"background" yaml:"background"` + TimeoutSec int `mapstructure:"timeout_sec" yaml:"timeout_sec"` +} +``` + +계획된 console request: + +```go +// apps/edge/cmd/edge/console.go:115 +req := &iop.RunRequest{ + RunId: runID, + Adapter: adapter, + Model: model, + SessionId: normalizeConsoleSessionID(sessionID), + SessionMode: iop.RunSessionMode_RUN_SESSION_MODE_CREATE_IF_MISSING, + Background: background, + Input: input, + TimeoutSec: int32(timeoutSec), + Metadata: map[string]string{ + "source": "edge-console", + }, +} +``` + +console 명령은 구현 범위를 작게 유지하되 manual testing을 위해 아래만 추가한다. + +```text +/session 현재 console target logical session 변경 +/background on|off 현재 console request background mode 변경 +/terminate-session 현재 adapter/model/session_id worker 종료 +``` + +`configs/edge.yaml` 예시는 `codex`를 persistent worker로 보여준다. 단, 사용자가 one-shot으로 계속 쓸 수 있도록 주석 또는 README에 `persistent: false` 선택지를 남긴다. + +```yaml +# configs/edge.yaml:14 +console: + adapter: "cli" + model: "codex" + session_id: "default" + background: false + timeout_sec: 120 + +# configs/edge.yaml:49 +codex: + command: "codex" + args: + - "exec" + - "--dangerously-bypass-approvals-and-sandbox" + - "--color" + - "never" + - "--skip-git-repo-check" + env: [] + persistent: true + terminal: false +``` + +#### 수정 파일 및 체크리스트 + +- [ ] `packages/config/config.go`: `EdgeConsoleConf.SessionID`, `EdgeConsoleConf.Background` 추가. CLI profile lifecycle field가 필요하면 zero default disabled 형태로 추가한다. `pool_size` 또는 hard limit field는 추가하지 않는다. +- [ ] `packages/config/config_test.go`: console defaults/override 테스트 갱신. +- [ ] `apps/edge/cmd/edge/console.go`: `/session`, `/background`, `/terminate-session` 명령과 `RunRequest` field 설정 추가. +- [ ] `apps/edge/internal/transport/server.go`: CLI settings payload에 새 config fields가 생기면 누락 없이 전달. node id당 one connection 거절 로직 `apps/edge/internal/transport/server.go:109`는 유지한다. +- [ ] `apps/edge/internal/transport/server_test.go`: 새 CLI profile fields roundtrip 검증. +- [ ] `configs/edge.yaml`: console session/background 예시와 codex persistent 예시 반영. +- [ ] `apps/node/README.md`, `apps/edge/README.md`: "transport connection 1개, logical session 여러 개" 사용법과 terminate/cancel 의미 차이를 문서화. + +#### 테스트 작성 + +작성한다. + +- `packages/config/config_test.go` + - `TestLoadEdge_ConsoleSessionDefaults`: default `session_id="default"`, `background=false` 확인. + - `TestLoadEdge_ConsoleSessionOverride`: YAML override 확인. +- `apps/edge/internal/transport/server_test.go` + - `TestBuildConfigPayload_CLISessionLifecycleRoundtrip`: 추가 config field가 structpb payload에 보존되는지 검증한다. +- `apps/edge/cmd/edge/console_test.go` + - 현재 없음. 새로 만들고 `sendConsoleRun`에 session/background 인자를 넣었을 때 `RunRequest`가 올바르게 만들어지는지 테스트한다. 직접 TCP client를 세우기 어렵다면 request-building helper를 추출해 단위 테스트한다. + +#### 중간 검증 + +```bash +go test ./packages/config ./apps/edge/internal/transport ./apps/edge/cmd/edge +``` + +예상 결과: config loading, edge config payload, edge console tests가 통과한다. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `proto/iop/runtime.proto` | REFACTOR-1 | +| `proto/gen/iop/runtime.pb.go` | REFACTOR-1 | +| `apps/node/internal/runtime/types.go` | REFACTOR-1, REFACTOR-2, REFACTOR-3 | +| `apps/node/internal/router/router.go` | REFACTOR-1 | +| `apps/node/internal/router/router_test.go` | REFACTOR-1 | +| `apps/node/internal/node/node.go` | REFACTOR-1, REFACTOR-2 | +| `apps/node/internal/node/run_manager.go` | REFACTOR-2 | +| `apps/node/internal/node/node_test.go` | REFACTOR-2 | +| `apps/node/internal/transport/session.go` | REFACTOR-2 | +| `apps/node/internal/transport/session_test.go` | REFACTOR-2 | +| `apps/node/internal/transport/parser_test.go` | REFACTOR-1 | +| `apps/node/internal/store/store.go` | REFACTOR-2 | +| `apps/node/internal/store/store_test.go` | REFACTOR-2 | +| `apps/node/internal/adapters/cli/cli.go` | REFACTOR-3 | +| `apps/node/internal/adapters/cli/persistent.go` | REFACTOR-3 | +| `apps/node/internal/adapters/cli/oneshot.go` | REFACTOR-3 | +| `apps/node/internal/adapters/cli/internal/testutil/testutil.go` | REFACTOR-3 | +| `apps/node/internal/adapters/cli/persistent/execute_test.go` | REFACTOR-3 | +| `apps/node/internal/adapters/cli/lifecycle/cli_test.go` | REFACTOR-3 | +| `packages/config/config.go` | REFACTOR-4 | +| `packages/config/config_test.go` | REFACTOR-4 | +| `apps/edge/cmd/edge/console.go` | REFACTOR-4 | +| `apps/edge/cmd/edge/console_test.go` | REFACTOR-4 | +| `apps/edge/internal/transport/server.go` | REFACTOR-4 | +| `apps/edge/internal/transport/server_test.go` | REFACTOR-4 | +| `configs/edge.yaml` | REFACTOR-4 | +| `apps/node/README.md` | REFACTOR-4 | +| `apps/edge/README.md` | REFACTOR-4 | + +## 콜사이트 및 호환성 체크 + +- `runtime.ExecutionSpec` 생성 콜사이트: + - `apps/node/internal/router/router.go:36` + - `apps/node/internal/node/node_test.go:43` + - `apps/node/internal/adapters/cli/persistent/execute_test.go:45` + - `apps/node/internal/adapters/cli/lifecycle/cli_test.go:75` +- `iop.RunRequest` 생성 콜사이트: + - `apps/edge/cmd/edge/console.go:115` + - `apps/node/internal/transport/parser_test.go:14` + - `apps/node/internal/transport/integration_test.go:139` + - `apps/node/internal/node/node_test.go:112` +- `iop.CancelRequest` 생성 콜사이트: + - `apps/node/internal/transport/parser_test.go:40` + - `apps/node/internal/node/node_test.go:143` +- 제거/이동 후보 `transport.Session` cancel methods 콜사이트: + - `apps/node/internal/node/node.go:95` + - `apps/node/internal/node/node.go:96` + - `apps/node/internal/node/node.go:118` + - `apps/node/internal/node/node_test.go:141` + - `apps/node/internal/transport/session_test.go:15` + +## 최종 검증 + +아래 명령을 순서대로 실행한다. + +```bash +make proto +gofmt -w \ + apps/node/internal/runtime/types.go \ + apps/node/internal/router/router.go \ + apps/node/internal/router/router_test.go \ + apps/node/internal/node/node.go \ + apps/node/internal/node/run_manager.go \ + apps/node/internal/node/node_test.go \ + apps/node/internal/transport/session.go \ + apps/node/internal/transport/session_test.go \ + apps/node/internal/transport/parser_test.go \ + apps/node/internal/store/store.go \ + apps/node/internal/store/store_test.go \ + apps/node/internal/adapters/cli/cli.go \ + apps/node/internal/adapters/cli/persistent.go \ + apps/node/internal/adapters/cli/oneshot.go \ + apps/node/internal/adapters/cli/internal/testutil/testutil.go \ + apps/node/internal/adapters/cli/persistent/execute_test.go \ + apps/node/internal/adapters/cli/lifecycle/cli_test.go \ + packages/config/config.go \ + packages/config/config_test.go \ + apps/edge/cmd/edge/console.go \ + apps/edge/cmd/edge/console_test.go \ + apps/edge/internal/transport/server.go \ + apps/edge/internal/transport/server_test.go +go test ./apps/node/internal/adapters/cli/... +go test ./apps/node/internal/runtime ./apps/node/internal/router ./apps/node/internal/node ./apps/node/internal/transport ./apps/node/internal/store +go test ./packages/config ./apps/edge/internal/transport ./apps/edge/cmd/edge +go test ./... +``` + +예상 결과: 모든 명령이 성공한다. `make proto`가 실패하면 `protoc` 또는 `protoc-gen-go` 설치 여부를 확인하되, 생성 파일을 직접 수정하지 않는다. diff --git a/agent-task/unbounded_cli_sessions/plan_1.log b/agent-task/unbounded_cli_sessions/plan_1.log new file mode 100644 index 0000000..4fe8feb --- /dev/null +++ b/agent-task/unbounded_cli_sessions/plan_1.log @@ -0,0 +1,221 @@ + + +# Follow-up Plan - Unbounded CLI Sessions Review Fixes + +## 이 파일을 읽는 구현 에이전트에게 + +이 계획은 `plan_0.log` / `code_review_0.log` 리뷰에서 발견된 Required 이슈만 해결하기 위한 follow-up입니다. 각 체크리스트를 완료하고, 중간/최종 검증을 실행한 뒤 `agent-task/unbounded_cli_sessions/CODE_REVIEW.md`의 모든 섹션에 실제 구현 메모와 명령 출력을 채우세요. + +구현 에이전트는 `CODE_REVIEW.md`의 "이 파일을 읽는 리뷰 에이전트에게" 섹션에 적힌 아카이브 지시를 실행하면 안 됩니다. `CODE_REVIEW.md` -> `code_review_*.log`, `PLAN.md` -> `plan_*.log`, `complete.log` 작성은 code-review skill 전용 작업입니다. + +## 배경 + +첫 구현은 unbounded CLI logical session의 큰 구조를 도입했지만, foreground adapter error 반환 계약과 기존 SQLite DB migration 안전성에서 Required 이슈가 남았습니다. 이 follow-up은 새 구조를 유지하면서 호출자 error 관측과 기존 workspace DB 호환성을 회복합니다. + +## 의존 관계 및 구현 순서 + +1. `REVIEW_REFACTOR-1`에서 node foreground error 반환 계약을 복구한다. +2. `REVIEW_REFACTOR-2`에서 store migration을 idempotent하게 추가한다. +3. 두 항목의 중간 검증 후 최종 `go test ./...`를 실행한다. + +### [REVIEW_REFACTOR-1] Restore Foreground OnRunRequest Error Return + +#### 문제 + +`OnRunRequest`의 foreground 경로가 adapter failure를 호출자에게 반환하지 않습니다. + +현재 코드: + +```go +// apps/node/internal/node/node.go:120 +run := func() { + defer cancel() + defer n.runs.deregister(spec.RunID) + defer close(h.done) + + execErr := adapter.Execute(execCtx, spec, sink) + n.completeRun(spec, execErr) +} + +if spec.Background { + go run() + return nil +} +run() +return nil +``` + +이전 계약과 `plan_0.log`의 pseudocode는 synchronous run에서 `adapter.Execute` error를 반환해야 합니다. 지금 상태에서는 store에는 failed/cancelled가 기록되더라도 transport handler caller는 실패를 알 수 없습니다. + +#### 해결 방법 + +`run` closure가 `error`를 반환하도록 바꾸고, foreground 경로에서는 그 error를 그대로 반환한다. background 경로는 기존처럼 goroutine dispatch 후 `nil`을 반환한다. + +계획 코드: + +```go +// apps/node/internal/node/node.go:120 +run := func() error { + defer cancel() + defer n.runs.deregister(spec.RunID) + defer close(h.done) + + execErr := adapter.Execute(execCtx, spec, sink) + n.completeRun(spec, execErr) + return execErr +} + +if spec.Background { + go func() { _ = run() }() + return nil +} +return run() +``` + +`runtime.ErrRunCancelled`도 adapter error이므로 foreground caller에 반환한다. store status는 기존 `completeRun` 로직대로 `cancelled`를 유지한다. + +#### 수정 파일 및 체크리스트 + +- [ ] `apps/node/internal/node/node.go`: `run` closure return type을 `error`로 변경. +- [ ] `apps/node/internal/node/node.go`: foreground path를 `return run()`으로 변경. +- [ ] `apps/node/internal/node/node.go`: background path는 `go func() { _ = run() }()`로 error를 소비하되 store update는 유지. +- [ ] `apps/node/internal/node/node_test.go`: adapter failure를 반환하는 test double 추가. +- [ ] `apps/node/internal/node/node_test.go`: foreground adapter failure가 `OnRunRequest` return error와 store `failed` status에 모두 반영되는 regression test 추가. +- [ ] `apps/node/internal/node/node_test.go`: foreground `runtime.ErrRunCancelled` 반환 시 `OnRunRequest`가 해당 sentinel error를 반환하고 store status가 `cancelled`가 되는지 필요하면 추가 test로 고정. + +#### 테스트 작성 + +작성한다. + +- `apps/node/internal/node/node_test.go` + - `TestOnRunRequest_ForegroundAdapterErrorReturned`: fake adapter가 `errors.New("adapter boom")`을 반환하게 하고 `OnRunRequest`가 non-nil error를 반환하는지, `st.GetRun(...).Status == "failed"`인지, `Error`에 `"adapter boom"`이 들어가는지 검증한다. + - `TestOnRunRequest_ForegroundCancelReturnedAndStored`: blocking adapter 또는 sentinel adapter를 사용해 foreground path에서 `runtime.ErrRunCancelled`가 반환되고 store status가 `cancelled`인지 검증한다. 구현이 복잡하면 첫 번째 test를 필수로 두고 cancelled path는 existing background cancel test로 충분한지 CODE_REVIEW에 근거를 남긴다. + +#### 중간 검증 + +```bash +go test ./apps/node/internal/node +``` + +예상 결과: node package tests가 통과하고 새 regression test가 실패 계약을 방지한다. + +### [REVIEW_REFACTOR-2] Add Idempotent Store Migration For Existing Runs Table + +#### 문제 + +store schema는 새 table 생성에는 `session_id`, `background` 컬럼을 포함하지만, 기존 DB에 이미 `runs` table이 있으면 `CREATE TABLE IF NOT EXISTS`는 아무 것도 바꾸지 않습니다. + +현재 코드: + +```go +// apps/node/internal/store/store.go:47 +func New(dsn string, logger *zap.Logger) (*Store, error) { + db, err := sql.Open("sqlite", dsn) + if err != nil { + return nil, fmt.Errorf("store: open %s: %w", dsn, err) + } + db.SetMaxOpenConns(1) // SQLite is single-writer + if _, err := db.Exec(schema); err != nil { + return nil, fmt.Errorf("store: migrate: %w", err) + } + logger.Info("store ready", zap.String("dsn", dsn)) + return &Store{db: db, logger: logger}, nil +} +``` + +그런데 `InsertRun`과 `GetRun`은 항상 새 컬럼을 사용합니다. + +```go +// apps/node/internal/store/store.go:77 +`INSERT INTO runs (run_id, adapter, model, session_id, background, status, created_at) VALUES (?,?,?,?,?,?,?)` + +// apps/node/internal/store/store.go:95 +`SELECT run_id, adapter, model, session_id, background, status, created_at, completed_at, error FROM runs WHERE run_id=?` +``` + +기존 workspace DB에서는 insert/select가 `no such column: session_id` 계열 오류로 깨집니다. + +#### 해결 방법 + +`schema` 실행 후 idempotent migration을 실행한다. SQLite는 `ADD COLUMN IF NOT EXISTS` 지원이 환경에 따라 애매할 수 있으므로, `PRAGMA table_info(runs)`로 컬럼 존재 여부를 확인한 뒤 없는 컬럼만 `ALTER TABLE` 한다. + +계획 코드: + +```go +// apps/node/internal/store/store.go:54 +if _, err := db.Exec(schema); err != nil { + return nil, fmt.Errorf("store: migrate: %w", err) +} +if err := migrateRunsTable(db); err != nil { + _ = db.Close() + return nil, fmt.Errorf("store: migrate runs: %w", err) +} +``` + +```go +// apps/node/internal/store/store.go:57 +func migrateRunsTable(db *sql.DB) error { + cols, err := tableColumns(db, "runs") + if err != nil { + return err + } + if !cols["session_id"] { + if _, err := db.Exec(`ALTER TABLE runs ADD COLUMN session_id TEXT NOT NULL DEFAULT 'default'`); err != nil { + return err + } + } + if !cols["background"] { + if _, err := db.Exec(`ALTER TABLE runs ADD COLUMN background INTEGER NOT NULL DEFAULT 0`); err != nil { + return err + } + } + return nil +} +``` + +`tableColumns`는 `PRAGMA table_info(runs)`의 `name` column을 읽어 `map[string]bool`로 반환한다. + +#### 수정 파일 및 체크리스트 + +- [ ] `apps/node/internal/store/store.go`: `migrateRunsTable` 추가. +- [ ] `apps/node/internal/store/store.go`: `tableColumns` helper 추가. +- [ ] `apps/node/internal/store/store.go`: migration 실패 시 DB close 후 error 반환. +- [ ] `apps/node/internal/store/store_test.go`: old schema fixture 생성 helper 추가. +- [ ] `apps/node/internal/store/store_test.go`: old schema DB를 `Store.New`로 열고 `InsertRun`/`GetRun`이 새 columns와 함께 동작하는 regression test 추가. + +#### 테스트 작성 + +작성한다. + +- `apps/node/internal/store/store_test.go` + - `TestStore_MigratesExistingRunsTable`: temp sqlite file에 구 schema(`run_id`, `adapter`, `model`, `status`, `created_at`, `completed_at`, `error`)만 가진 `runs` table을 먼저 만든다. 이후 `store.New(dsn, ...)`를 호출하고 `InsertRun` with `SessionID: "session-a", Background: true`가 성공하는지, `GetRun`이 해당 값을 읽는지 검증한다. + - `TestStore_MigrationIsIdempotent`: 같은 DB에 `store.New`를 두 번 호출해 두 번째 호출도 실패하지 않는지 검증한다. + +#### 중간 검증 + +```bash +go test ./apps/node/internal/store +``` + +예상 결과: store package tests가 통과하고 구 schema DB migration regression test가 추가된다. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `apps/node/internal/node/node.go` | REVIEW_REFACTOR-1 | +| `apps/node/internal/node/node_test.go` | REVIEW_REFACTOR-1 | +| `apps/node/internal/store/store.go` | REVIEW_REFACTOR-2 | +| `apps/node/internal/store/store_test.go` | REVIEW_REFACTOR-2 | +| `agent-task/unbounded_cli_sessions/CODE_REVIEW.md` | REVIEW_REFACTOR-1, REVIEW_REFACTOR-2 | + +## 최종 검증 + +```bash +gofmt -w apps/node/internal/node/node.go apps/node/internal/node/node_test.go apps/node/internal/store/store.go apps/node/internal/store/store_test.go +go test ./apps/node/internal/node +go test ./apps/node/internal/store +go test ./... +``` + +예상 결과: 모든 명령이 성공한다. diff --git a/apps/edge/README.md b/apps/edge/README.md index 3544373..525cc58 100644 --- a/apps/edge/README.md +++ b/apps/edge/README.md @@ -18,13 +18,13 @@ node 등록/레지스트리/transport와 edge 콘솔 기반 수동 통신 테스 실행 순서: -1. edge 호스트에서 `configs/edge.yaml`의 `server.listen`, `nodes[].token`을 확인한다. `console.adapter=cli`, `console.model=codex`가 기본값이다. +1. edge 호스트에서 `configs/edge.yaml`의 `server.listen`, `nodes[].token`을 확인한다. `console.adapter=cli`, `console.model=codex`, `console.session_id=default`가 기본값이다. 2. node 호스트에서 `configs/node.yaml`의 `transport.edge_addr`를 edge 호스트 주소로, `transport.token`을 edge token과 같게 맞춘다. 3. edge 호스트의 `9090/tcp` 포트를 node 호스트에서 접근 가능하게 연다. 4. edge 호스트에서 `./bin/edge.sh` -5. node 호스트에서 `./bin/node.sh` — node는 edge에서 받은 입력을 `codex exec --dangerously-bypass-approvals-and-sandbox` one-shot process로 실행한다. +5. node 호스트에서 `./bin/node.sh` — node는 edge에서 받은 입력을 `codex exec --dangerously-bypass-approvals-and-sandbox` persistent process로 실행한다. 6. edge 콘솔에서 `/nodes`로 node 등록을 확인한다. -7. edge 콘솔의 `edge>` 프롬프트에 메시지를 입력한다. 입력은 node의 Codex process prompt 인자로 전달된다. +7. edge 콘솔의 `edge>` 프롬프트에 메시지를 입력한다. 8. `[node-{alias}-event]` 라인으로 실행 상태를, `[node-{alias}-message]` 라인으로 Codex 응답을 확인한다. 9. edge 콘솔에서 `/exit` 또는 `quit`로 종료한다. @@ -32,12 +32,29 @@ node 등록/레지스트리/transport와 edge 콘솔 기반 수동 통신 테스 ```text edge> hello -[edge] sent run_id=manual-... node=local-node adapter=cli model=codex +[edge] sent run_id=manual-... node=local-node adapter=cli model=codex session=default background=false [node-local-node-event] start run_id=manual-... [node-local-node-event] complete run_id=manual-... detail="cli execution complete" [node-local-node-message] ``` +## Console 명령 + +| 명령 | 설명 | +|---|---| +| `/session ` | 현재 console이 사용할 logical session 변경 | +| `/background on\|off` | background 실행 모드 토글 (on: 응답 기다리지 않음) | +| `/terminate-session` | 현재 `adapter/model/session_id`의 worker process 종료 | +| `/nodes` | 연결된 node 목록 확인 | +| `/exit` | 콘솔 종료 | + +## Transport 1개 · Logical Session 여러 개 + +edge-node transport 연결은 **node id당 1개** TCP 연결만 유지한다. 그 연결 위에서 edge는 `session_id`를 지정해 node의 CLI adapter가 관리하는 개별 worker process에 접근한다. 같은 `codex` profile이라도 `session_id`가 다르면 독립적인 장수 process다. + +- **cancel run**: 현재 run 중단, session process 유지 +- **terminate session**: session process 명시 종료 (`/terminate-session` 또는 `CancelAction_TERMINATE_SESSION`) + 프롬프트 없이 TCP/protobuf edge 서버만 실행하려면 기존처럼 `go run ./apps/edge/cmd/edge serve -c configs/edge.yaml`를 사용한다. ## 계획된 기능 diff --git a/apps/edge/cmd/edge/console.go b/apps/edge/cmd/edge/console.go index 9b3cb0c..a79ad02 100644 --- a/apps/edge/cmd/edge/console.go +++ b/apps/edge/cmd/edge/console.go @@ -55,10 +55,17 @@ func runConsole(ctx context.Context, cfg *config.EdgeConfig, in io.Reader, out i } }() + // Console state: adapter, model, session, background mode. + adapter := cfg.Console.Adapter + model := cfg.Console.Model + sessionID := normalizeConsoleSessionID(cfg.Console.SessionID) + background := cfg.Console.Background + timeoutSec := cfg.Console.TimeoutSec + fmt.Fprintf(out, "IOP Edge console listening on %s\n", cfg.Server.Listen) - fmt.Fprintf(out, "Console target adapter=%s model=%s\n", cfg.Console.Adapter, cfg.Console.Model) + fmt.Fprintf(out, "Console target adapter=%s model=%s session=%s background=%v\n", adapter, model, sessionID, background) fmt.Fprintln(out, "Start node.sh on another host, then type a message here.") - fmt.Fprintln(out, "Commands: /nodes, /exit") + fmt.Fprintln(out, "Commands: /nodes, /session , /background on|off, /terminate-session, /exit") scanner := bufio.NewScanner(in) for { @@ -69,16 +76,47 @@ func runConsole(ctx context.Context, cfg *config.EdgeConfig, in io.Reader, out i } message := strings.TrimSpace(scanner.Text()) - switch strings.ToLower(message) { - case "": + lower := strings.ToLower(message) + switch { + case message == "": continue - case "/exit", "/quit", "exit", "quit": + case lower == "/exit" || lower == "/quit" || lower == "exit" || lower == "quit": fmt.Fprintln(out, "bye") return nil - case "/nodes": + case lower == "/nodes": printNodes(out, registry) + case strings.HasPrefix(lower, "/session "): + parts := strings.Fields(message) + if len(parts) < 2 || parts[1] == "" { + fmt.Fprintln(out, "usage: /session ") + continue + } + sessionID = parts[1] + fmt.Fprintf(out, "session → %s\n", sessionID) + case strings.HasPrefix(lower, "/background "): + parts := strings.Fields(lower) + if len(parts) < 2 { + fmt.Fprintln(out, "usage: /background on|off") + continue + } + switch parts[1] { + case "on": + background = true + fmt.Fprintln(out, "background → on") + case "off": + background = false + fmt.Fprintln(out, "background → off") + default: + fmt.Fprintln(out, "usage: /background on|off") + } + case lower == "/terminate-session": + if err := sendTerminateSession(ctx, registry, adapter, model, sessionID); err != nil { + fmt.Fprintf(out, "error: %v\n", err) + } else { + fmt.Fprintf(out, "terminated session %s\n", sessionID) + } default: - if err := sendConsoleRun(ctx, registry, events, out, cfg.Console.Adapter, cfg.Console.Model, cfg.Console.TimeoutSec, message); err != nil { + if err := sendConsoleRun(ctx, registry, events, out, adapter, model, sessionID, background, timeoutSec, message); err != nil { fmt.Fprintf(out, "error: %v\n", err) } } @@ -96,39 +134,55 @@ func printNodes(out io.Writer, registry *edgenode.Registry) { } } -func sendConsoleRun(ctx context.Context, registry *edgenode.Registry, events <-chan *iop.RunEvent, out io.Writer, adapter, model string, timeoutSec int, message string) error { +// buildRunRequest constructs the RunRequest for a console send. +func buildRunRequest(adapter, model, sessionID string, background bool, timeoutSec int, message string) (*iop.RunRequest, string, error) { + input, err := structpb.NewStruct(map[string]any{"prompt": message}) + if err != nil { + return nil, "", err + } + if timeoutSec <= 0 { + timeoutSec = 30 + } + runID := fmt.Sprintf("manual-%d", time.Now().UnixNano()) + req := &iop.RunRequest{ + RunId: runID, + Adapter: adapter, + Model: model, + SessionId: normalizeConsoleSessionID(sessionID), + SessionMode: iop.RunSessionMode_RUN_SESSION_MODE_CREATE_IF_MISSING, + Background: background, + Input: input, + TimeoutSec: int32(timeoutSec), + Metadata: map[string]string{ + "source": "edge-console", + }, + } + return req, runID, nil +} + +func sendConsoleRun(ctx context.Context, registry *edgenode.Registry, events <-chan *iop.RunEvent, out io.Writer, adapter, model, sessionID string, background bool, timeoutSec int, message string) error { entry, err := registry.Pick() if err != nil { return err } - input, err := structpb.NewStruct(map[string]any{"prompt": message}) + req, runID, err := buildRunRequest(adapter, model, sessionID, background, timeoutSec, message) if err != nil { return err } - if timeoutSec <= 0 { - timeoutSec = 30 - } - - runID := fmt.Sprintf("manual-%d", time.Now().UnixNano()) - req := &iop.RunRequest{ - RunId: runID, - Adapter: adapter, - Model: model, - Input: input, - TimeoutSec: int32(timeoutSec), - Metadata: map[string]string{ - "source": "edge-console", - }, - } - nodeAlias := entry.Alias - fmt.Fprintf(out, "[edge] sent run_id=%s node=%s adapter=%s model=%s\n", runID, nodeAlias, adapter, model) + fmt.Fprintf(out, "[edge] sent run_id=%s node=%s adapter=%s model=%s session=%s background=%v\n", + runID, nodeAlias, adapter, model, req.GetSessionId(), background) if err := entry.Client.Send(req); err != nil { return err } + if background { + fmt.Fprintf(out, "[edge] background run dispatched, events will arrive asynchronously\n") + return nil + } + timer := time.NewTimer(time.Duration(timeoutSec+5) * time.Second) defer timer.Stop() var response strings.Builder @@ -152,6 +206,9 @@ func sendConsoleRun(ctx context.Context, registry *edgenode.Registry, events <-c fmt.Fprintf(out, "[node-%s-event] complete run_id=%s detail=%q\n", nodeAlias, runID, event.GetMessage()) printNodeMessage(out, nodeAlias, response.String()) return nil + case "cancelled": + fmt.Fprintf(out, "[node-%s-event] cancelled run_id=%s\n", nodeAlias, runID) + return nil case "error": fmt.Fprintf(out, "[node-%s-event] error run_id=%s detail=%q\n", nodeAlias, runID, event.GetError()) printNodeMessage(out, nodeAlias, response.String()) @@ -163,6 +220,20 @@ func sendConsoleRun(ctx context.Context, registry *edgenode.Registry, events <-c } } +func sendTerminateSession(ctx context.Context, registry *edgenode.Registry, adapter, model, sessionID string) error { + entry, err := registry.Pick() + if err != nil { + return err + } + req := &iop.CancelRequest{ + Adapter: adapter, + Model: model, + SessionId: normalizeConsoleSessionID(sessionID), + Action: iop.CancelAction_CANCEL_ACTION_TERMINATE_SESSION, + } + return entry.Client.Send(req) +} + func printNodeMessage(out io.Writer, alias, message string) { message = strings.TrimSpace(message) if message == "" { @@ -171,3 +242,10 @@ func printNodeMessage(out io.Writer, alias, message string) { } fmt.Fprintf(out, "[node-%s-message] %s\n", alias, message) } + +func normalizeConsoleSessionID(id string) string { + if id == "" { + return "default" + } + return id +} diff --git a/apps/edge/cmd/edge/console_test.go b/apps/edge/cmd/edge/console_test.go new file mode 100644 index 0000000..8a0c229 --- /dev/null +++ b/apps/edge/cmd/edge/console_test.go @@ -0,0 +1,61 @@ +package main + +import ( + "testing" + + iop "iop/proto/gen/iop" +) + +func TestBuildRunRequest_SessionAndBackground(t *testing.T) { + req, runID, err := buildRunRequest("cli", "codex", "session-a", true, 30, "hello") + if err != nil { + t.Fatalf("buildRunRequest: %v", err) + } + if runID == "" { + t.Fatal("expected non-empty runID") + } + if req.GetAdapter() != "cli" { + t.Errorf("Adapter: got %q want %q", req.GetAdapter(), "cli") + } + if req.GetModel() != "codex" { + t.Errorf("Model: got %q want %q", req.GetModel(), "codex") + } + if req.GetSessionId() != "session-a" { + t.Errorf("SessionId: got %q want %q", req.GetSessionId(), "session-a") + } + if !req.GetBackground() { + t.Error("Background: expected true") + } + if req.GetSessionMode() != iop.RunSessionMode_RUN_SESSION_MODE_CREATE_IF_MISSING { + t.Errorf("SessionMode: got %v", req.GetSessionMode()) + } +} + +func TestBuildRunRequest_EmptySessionNormalized(t *testing.T) { + req, _, err := buildRunRequest("cli", "codex", "", false, 30, "hello") + if err != nil { + t.Fatalf("buildRunRequest: %v", err) + } + if req.GetSessionId() != "default" { + t.Errorf("SessionId: got %q want %q", req.GetSessionId(), "default") + } +} + +func TestBuildRunRequest_DefaultTimeoutFallback(t *testing.T) { + req, _, err := buildRunRequest("cli", "codex", "s1", false, 0, "hello") + if err != nil { + t.Fatalf("buildRunRequest: %v", err) + } + if req.GetTimeoutSec() != 30 { + t.Errorf("TimeoutSec: got %d want 30", req.GetTimeoutSec()) + } +} + +func TestNormalizeConsoleSessionID(t *testing.T) { + if got := normalizeConsoleSessionID(""); got != "default" { + t.Errorf("empty: got %q want %q", got, "default") + } + if got := normalizeConsoleSessionID("my-session"); got != "my-session" { + t.Errorf("non-empty: got %q want %q", got, "my-session") + } +} diff --git a/apps/node/README.md b/apps/node/README.md index 33cfe86..14c8103 100644 --- a/apps/node/README.md +++ b/apps/node/README.md @@ -45,7 +45,7 @@ go build -o bin/node ./apps/node/cmd/node edge의 기본 설정은 cli adapter의 `codex` profile을 사용한다. PATH에 `codex` 명령이 있어야 하며, 각 실행 요청은 `codex exec --dangerously-bypass-approvals-and-sandbox` one-shot process로 처리된다. -edge에서 실행 요청을 받으면 node는 해당 입력을 Codex process의 prompt 인자로 전달하고, process 종료 시 complete 이벤트를 보낸다. +edge에서 실행 요청을 받으면 node는 해당 입력을 Codex process의 prompt 인자로 전달하고, complete 이벤트를 보낸다. ```text [edge-message] hello @@ -54,6 +54,26 @@ edge에서 실행 요청을 받으면 node는 해당 입력을 Codex process의 [node-message] ``` +## Logical Session (transport 1개 · session 여러 개) + +edge-node transport 연결은 **호스트당 1개**를 유지한다. 그 연결 위에서 CLI adapter는 `session_id`가 다른 여러 장수 worker process를 독립적으로 관리한다. + +| 개념 | 설명 | +|---|---| +| transport 연결 | edge-node 호스트 쌍당 1개 TCP 연결 | +| logical session | `(adapter, model, session_id)` 로 식별되는 장수 worker process | +| run | session 위에서 실행되는 단일 요청 | + +**cancel vs terminate:** + +- `CancelAction_CANCEL_RUN` (기본값): 현재 실행 중인 run만 중단. session process는 살아있다. +- `CancelAction_TERMINATE_SESSION`: session process를 명시적으로 종료. 이후 같은 `session_id`로의 요청은 새 process를 만들거나(`CREATE_IF_MISSING`) 에러를 반환한다(`REQUIRE_EXISTING`). + +**session_mode:** + +- `RUN_SESSION_MODE_CREATE_IF_MISSING` (기본값): session이 없으면 새로 생성. +- `RUN_SESSION_MODE_REQUIRE_EXISTING`: session이 없으면 에러 반환. 새 process 생성 금지. + ## 어댑터 | 어댑터 | 설명 | 상태 | diff --git a/apps/node/internal/adapters/cli/cli.go b/apps/node/internal/adapters/cli/cli.go index d08201f..1fb003f 100644 --- a/apps/node/internal/adapters/cli/cli.go +++ b/apps/node/internal/adapters/cli/cli.go @@ -28,7 +28,14 @@ type cliOutput struct { line string } +// sessionKey uniquely identifies a logical worker session. +type sessionKey struct { + model string + sessionID string +} + type profileSession struct { + key sessionKey name string profile config.CLIProfileConf cmd *exec.Cmd @@ -42,14 +49,14 @@ type profileSession struct { type CLI struct { mu sync.Mutex profiles map[string]config.CLIProfileConf - sessions map[string]*profileSession + sessions map[sessionKey]*profileSession logger *zap.Logger } func New(cfg config.CLIConf, logger *zap.Logger) *CLI { return &CLI{ profiles: cfg.Profiles, - sessions: make(map[string]*profileSession), + sessions: make(map[sessionKey]*profileSession), logger: logger, } } @@ -61,7 +68,6 @@ func (c *CLI) Capabilities(_ context.Context) (runtime.Capabilities, error) { for name := range c.profiles { models = append(models, name) } - // Always advertise known profiles even if not in config yet. for _, p := range knownProfiles { if _, ok := c.profiles[p]; !ok { models = append(models, p) @@ -74,10 +80,9 @@ func (c *CLI) Capabilities(_ context.Context) (runtime.Capabilities, error) { }, nil } -// Start starts persistent profile sessions in deterministic (sorted) order. +// Start starts the default session for each persistent profile in deterministic (sorted) order. // On failure, already-started sessions are rolled back under c.mu. func (c *CLI) Start(ctx context.Context) error { - // Sort profile names for deterministic start order. names := make([]string, 0, len(c.profiles)) for name := range c.profiles { if c.profiles[name].Persistent { @@ -88,17 +93,17 @@ func (c *CLI) Start(ctx context.Context) error { for _, name := range names { profile := c.profiles[name] - sess, err := startProfileSession(ctx, name, profile, c.logger) + key := sessionKey{model: name, sessionID: runtime.DefaultSessionID} + sess, err := startProfileSession(ctx, key, profile, c.logger) if err != nil { - // Roll back already-started sessions under the lock. c.mu.Lock() _ = c.stopAllSessions(context.Background()) - c.sessions = make(map[string]*profileSession) + c.sessions = make(map[sessionKey]*profileSession) c.mu.Unlock() return fmt.Errorf("cli adapter: start profile %q: %w", name, err) } c.mu.Lock() - c.sessions[name] = sess + c.sessions[key] = sess c.mu.Unlock() c.logger.Info("cli adapter: persistent session started", zap.String("profile", name)) } @@ -109,39 +114,29 @@ func (c *CLI) Start(ctx context.Context) error { // Must be called while c.mu is held. func (c *CLI) stopAllSessions(_ context.Context) error { var firstErr error - for name, sess := range c.sessions { - if err := sess.closeFn(); err != nil && firstErr == nil { - firstErr = fmt.Errorf("cli adapter: close session %q: %w", name, err) - } - if sess.cmd != nil && sess.cmd.Process != nil { - _ = sess.cmd.Process.Kill() + for key, sess := range c.sessions { + if err := closeProfileSession(context.Background(), sess); err != nil && firstErr == nil { + firstErr = fmt.Errorf("cli adapter: close session %q/%q: %w", key.model, key.sessionID, err) } } - c.sessions = make(map[string]*profileSession) + c.sessions = make(map[sessionKey]*profileSession) return firstErr } -// Stop stops all persistent sessions. Sessions are iterated in map order (non-deterministic), -// but since Stop is idempotent and all sessions are stopped regardless of individual failures, -// the order does not affect correctness. Errors are combined by reporting only the first error. +// Stop stops all logical sessions. Errors are combined by reporting only the first. func (c *CLI) Stop(_ context.Context) error { c.mu.Lock() - // Snapshot sessions under the lock, then release it before doing - // blocking close/kill so we don't hold c.mu while processes terminate. - sessionsCopy := make(map[string]*profileSession, len(c.sessions)) - for name, sess := range c.sessions { - sessionsCopy[name] = sess + sessionsCopy := make(map[sessionKey]*profileSession, len(c.sessions)) + for key, sess := range c.sessions { + sessionsCopy[key] = sess } - c.sessions = make(map[string]*profileSession) + c.sessions = make(map[sessionKey]*profileSession) c.mu.Unlock() var firstErr error - for name, sess := range sessionsCopy { - if err := sess.closeFn(); err != nil && firstErr == nil { - firstErr = fmt.Errorf("cli adapter: close session %q: %w", name, err) - } - if sess.cmd != nil && sess.cmd.Process != nil { - _ = sess.cmd.Process.Kill() + for key, sess := range sessionsCopy { + if err := closeProfileSession(context.Background(), sess); err != nil && firstErr == nil { + firstErr = fmt.Errorf("cli adapter: close session %q/%q: %w", key.model, key.sessionID, err) } } return firstErr @@ -157,3 +152,33 @@ func (c *CLI) Execute(ctx context.Context, spec runtime.ExecutionSpec, sink runt } return c.executeOneShot(ctx, spec, profile, sink) } + +// TerminateSession implements runtime.SessionTerminator. +func (c *CLI) TerminateSession(_ context.Context, model, sessionID string) error { + key := sessionKey{model: model, sessionID: normalizeSessionID(sessionID)} + c.mu.Lock() + sess, ok := c.sessions[key] + if ok { + delete(c.sessions, key) + } + c.mu.Unlock() + if !ok { + return fmt.Errorf("cli adapter: no session %q for profile %q", key.sessionID, model) + } + return closeProfileSession(context.Background(), sess) +} + +func normalizeSessionID(id string) string { + if id == "" { + return runtime.DefaultSessionID + } + return id +} + +func closeProfileSession(_ context.Context, sess *profileSession) error { + err := sess.closeFn() + if sess.cmd != nil && sess.cmd.Process != nil { + _ = sess.cmd.Process.Kill() + } + return err +} diff --git a/apps/node/internal/adapters/cli/lifecycle/cli_test.go b/apps/node/internal/adapters/cli/lifecycle/cli_test.go index 51b011f..bdb1041 100644 --- a/apps/node/internal/adapters/cli/lifecycle/cli_test.go +++ b/apps/node/internal/adapters/cli/lifecycle/cli_test.go @@ -70,14 +70,17 @@ func TestCLIStartCleansUpStartedProfilesWhenLaterProfileFails(t *testing.T) { _ = c.Stop(ctx) _ = c.Stop(ctx) + // After rollback, the pre-started default session for ok-profile is gone. + // Verify with require-existing so lazy creation is not allowed. for _, model := range []string{"ok-profile", "fail-profile"} { sink := &testutil.FakeSink{} if err := c.Execute(ctx, noderuntime.ExecutionSpec{ - RunID: "post-cleanup", - Model: model, - Input: map[string]any{"prompt": "test"}, + RunID: "post-cleanup", + Model: model, + SessionMode: noderuntime.SessionModeRequireExisting, + Input: map[string]any{"prompt": "test"}, }, sink); err == nil { - t.Fatalf("expected Execute for %q to fail after cleanup, got nil", model) + t.Fatalf("expected Execute for %q to fail after cleanup (require-existing), got nil", model) } } } @@ -115,14 +118,16 @@ func TestCLIStartDeterministicOrder(t *testing.T) { _ = c.Stop(ctx) _ = c.Stop(ctx) + // After rollback, pre-started default sessions must be gone. for _, model := range []string{"Z-alive", "Z-fail"} { sink := &testutil.FakeSink{} if err := c.Execute(ctx, noderuntime.ExecutionSpec{ - RunID: "post-cleanup", - Model: model, - Input: map[string]any{"prompt": "test"}, + RunID: "post-cleanup", + Model: model, + SessionMode: noderuntime.SessionModeRequireExisting, + Input: map[string]any{"prompt": "test"}, }, sink); err == nil { - t.Fatalf("expected Execute for %q to fail after cleanup, got nil", model) + t.Fatalf("expected Execute for %q to fail after cleanup (require-existing), got nil", model) } } } @@ -250,12 +255,130 @@ func TestCLIStartPartialRollbackWithMarkers(t *testing.T) { return testutil.ReadMarker(startMarkerPath) == startMarker }) + // After rollback, the pre-started default session for profile-a is gone. sink := &testutil.FakeSink{} if err := c.Execute(ctx, noderuntime.ExecutionSpec{ - RunID: "post-rollback", - Model: "profile-a", - Input: map[string]any{"prompt": "test"}, + RunID: "post-rollback", + Model: "profile-a", + SessionMode: noderuntime.SessionModeRequireExisting, + Input: map[string]any{"prompt": "test"}, }, sink); err == nil { - t.Fatal("expected Execute for profile-a to fail with no-session after rollback") + t.Fatal("expected Execute for profile-a to fail with no-session after rollback (require-existing)") + } +} + +func TestCLITerminateSessionStopsOnlyTargetSession(t *testing.T) { + testutil.RequireUnixShell(t) + + cfg := config.CLIConf{ + Enabled: true, + Profiles: map[string]config.CLIProfileConf{ + "echo-term": { + Command: "sh", + Args: []string{"-c", `while IFS= read -r line; do printf "reply:%s\n" "$line"; done`}, + Persistent: true, + Terminal: false, + ResponseIdleTimeoutMS: 200, + }, + }, + } + c := clipkg.New(cfg, zap.NewNop()) + ctx := context.Background() + + // Create session-a and session-b via lazy creation. + for _, sid := range []string{"session-a", "session-b"} { + sink := &testutil.FakeSink{} + err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-" + sid, + Model: "echo-term", + SessionID: sid, + Input: map[string]any{"prompt": "ping"}, + }, sink) + if err != nil { + t.Fatalf("create %s: %v", sid, err) + } + } + + // Terminate session-a only. + if err := c.TerminateSession(ctx, "echo-term", "session-a"); err != nil { + t.Fatalf("TerminateSession: %v", err) + } + + // session-b must still respond. + sink := &testutil.FakeSink{} + err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-b-after", + Model: "echo-term", + SessionID: "session-b", + Input: map[string]any{"prompt": "pong"}, + }, sink) + if err != nil { + t.Fatalf("session-b should still be alive: %v", err) + } + + // session-a must be gone (require-existing fails). + sinkA := &testutil.FakeSink{} + err = c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-a-after", + Model: "echo-term", + SessionID: "session-a", + SessionMode: noderuntime.SessionModeRequireExisting, + Input: map[string]any{"prompt": "should-fail"}, + }, sinkA) + if err == nil { + t.Fatal("expected error for terminated session-a with require-existing mode") + } + + _ = c.Stop(ctx) +} + +func TestCLIStopStopsAllLogicalSessions(t *testing.T) { + testutil.RequireUnixShell(t) + + cfg := config.CLIConf{ + Enabled: true, + Profiles: map[string]config.CLIProfileConf{ + "echo-stop": { + Command: "sh", + Args: []string{"-c", `while IFS= read -r line; do printf "reply:%s\n" "$line"; done`}, + Persistent: true, + Terminal: false, + ResponseIdleTimeoutMS: 200, + }, + }, + } + c := clipkg.New(cfg, zap.NewNop()) + ctx := context.Background() + + // Create three logical sessions. + for _, sid := range []string{"s1", "s2", "s3"} { + sink := &testutil.FakeSink{} + if err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-" + sid, + Model: "echo-stop", + SessionID: sid, + Input: map[string]any{"prompt": "ping"}, + }, sink); err != nil { + t.Fatalf("create %s: %v", sid, err) + } + } + + if err := c.Stop(ctx); err != nil { + t.Fatalf("Stop: %v", err) + } + + // After Stop, all sessions must be gone (require-existing must fail). + for _, sid := range []string{"s1", "s2", "s3"} { + sink := &testutil.FakeSink{} + err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-after-" + sid, + Model: "echo-stop", + SessionID: sid, + SessionMode: noderuntime.SessionModeRequireExisting, + Input: map[string]any{"prompt": "should-fail"}, + }, sink) + if err == nil { + t.Fatalf("session %q should be stopped after Stop()", sid) + } } } diff --git a/apps/node/internal/adapters/cli/persistent.go b/apps/node/internal/adapters/cli/persistent.go index a940277..e74db06 100644 --- a/apps/node/internal/adapters/cli/persistent.go +++ b/apps/node/internal/adapters/cli/persistent.go @@ -27,11 +27,9 @@ func emitRuntimeError(ctx context.Context, sink runtime.EventSink, runID, msg st } func (c *CLI) executePersistent(ctx context.Context, spec runtime.ExecutionSpec, profile config.CLIProfileConf, sink runtime.EventSink) error { - c.mu.Lock() - sess, ok := c.sessions[spec.Model] - c.mu.Unlock() - if !ok { - return fmt.Errorf("cli adapter: no persistent session for profile %q", spec.Model) + sess, err := c.resolveSession(ctx, spec, profile) + if err != nil { + return err } sess.mu.Lock() @@ -65,18 +63,23 @@ func (c *CLI) executePersistent(ctx context.Context, spec runtime.ExecutionSpec, for { select { case <-ctx.Done(): - c.mu.Lock() - if s, ok := c.sessions[spec.Model]; ok && s == sess { - _ = s.closeFn() - if s.cmd != nil && s.cmd.Process != nil { - _ = s.cmd.Process.Kill() - } - delete(c.sessions, spec.Model) - } - c.mu.Unlock() - return ctx.Err() + // Drain output so the process remains usable for the next run. + drainSessionUntilIdle(sess.output, idleTimeout, c.logger, sess.key) + _ = sink.Emit(context.Background(), runtime.RuntimeEvent{ + RunID: spec.RunID, + Type: runtime.EventTypeCancelled, + Message: "cli execution cancelled", + Timestamp: time.Now(), + }) + return runtime.ErrRunCancelled case out, ok := <-sess.output: if !ok { + // Process exited — remove the dead session. + c.mu.Lock() + if s, found := c.sessions[sess.key]; found && s == sess { + delete(c.sessions, sess.key) + } + c.mu.Unlock() return emitRuntimeError(ctx, sink, spec.RunID, "persistent session process exited unexpectedly") } outputTokens += len(strings.Fields(out.line)) @@ -110,6 +113,11 @@ func (c *CLI) executePersistent(ctx context.Context, spec runtime.ExecutionSpec, Timestamp: time.Now(), }) case err := <-sess.done: + c.mu.Lock() + if s, found := c.sessions[sess.key]; found && s == sess { + delete(c.sessions, sess.key) + } + c.mu.Unlock() msg := "persistent session process exited" if err != nil { msg = fmt.Sprintf("persistent session process exited: %v", err) @@ -119,9 +127,30 @@ func (c *CLI) executePersistent(ctx context.Context, spec runtime.ExecutionSpec, } } -func startProfileSession(_ context.Context, name string, profile config.CLIProfileConf, logger *zap.Logger) (*profileSession, error) { +// resolveSession returns an existing session for the given key or creates one +// when SessionMode allows it. +func (c *CLI) resolveSession(ctx context.Context, spec runtime.ExecutionSpec, profile config.CLIProfileConf) (*profileSession, error) { + key := sessionKey{model: spec.Model, sessionID: normalizeSessionID(spec.SessionID)} + c.mu.Lock() + defer c.mu.Unlock() + + if sess, ok := c.sessions[key]; ok { + return sess, nil + } + if spec.SessionMode == runtime.SessionModeRequireExisting { + return nil, fmt.Errorf("cli adapter: no persistent session for profile %q session %q", spec.Model, key.sessionID) + } + sess, err := startProfileSession(ctx, key, profile, c.logger) + if err != nil { + return nil, err + } + c.sessions[key] = sess + return sess, nil +} + +func startProfileSession(_ context.Context, key sessionKey, profile config.CLIProfileConf, logger *zap.Logger) (*profileSession, error) { if profile.Command == "" { - return nil, fmt.Errorf("profile %q has no command", name) + return nil, fmt.Errorf("profile %q has no command", key.model) } outputCh := make(chan cliOutput, 1024) @@ -182,7 +211,7 @@ func startProfileSession(_ context.Context, name string, profile config.CLIProfi } if profile.StartupIdleTimeoutMS > 0 { - drainUntilIdle(outputCh, time.Duration(profile.StartupIdleTimeoutMS)*time.Millisecond, logger, name) + drainUntilIdle(outputCh, time.Duration(profile.StartupIdleTimeoutMS)*time.Millisecond, logger, key.model) } select { @@ -199,7 +228,8 @@ func startProfileSession(_ context.Context, name string, profile config.CLIProfi } return &profileSession{ - name: name, + key: key, + name: key.model, profile: profile, cmd: cmd, input: input, @@ -231,3 +261,26 @@ func drainUntilIdle(outputCh <-chan cliOutput, timeout time.Duration, logger *za } } } + +func drainSessionUntilIdle(outputCh <-chan cliOutput, timeout time.Duration, logger *zap.Logger, key sessionKey) { + timer := time.NewTimer(timeout) + defer timer.Stop() + for { + select { + case _, ok := <-outputCh: + if !ok { + return + } + if !timer.Stop() { + select { + case <-timer.C: + default: + } + } + timer.Reset(timeout) + logger.Debug("cli adapter: cancel drain", zap.String("profile", key.model), zap.String("session", key.sessionID)) + case <-timer.C: + return + } + } +} diff --git a/apps/node/internal/adapters/cli/persistent/execute_test.go b/apps/node/internal/adapters/cli/persistent/execute_test.go index e74df84..d35369d 100644 --- a/apps/node/internal/adapters/cli/persistent/execute_test.go +++ b/apps/node/internal/adapters/cli/persistent/execute_test.go @@ -164,18 +164,22 @@ func TestCLIStartPersistentTerminalAndExecuteWritesPrompt(t *testing.T) { } } -func TestCLIExecutePersistentTimeoutCleansSession(t *testing.T) { +// TestCLIExecutePersistentCancelDoesNotTerminateSession verifies that a +// cancelled run leaves the session alive so subsequent runs can reuse it. +func TestCLIExecutePersistentCancelDoesNotTerminateSession(t *testing.T) { testutil.RequirePTYSupport(t) + // ResponseIdleTimeoutMS is short so the cancel-drain exits quickly, + // allowing the second execute to acquire the session lock promptly. cfg := config.CLIConf{ Enabled: true, Profiles: map[string]config.CLIProfileConf{ "slow-echo": { Command: "sh", - Args: []string{"-c", `stty -echo; while IFS= read -r line; do sleep 0.1; printf "reply:%s\n" "$line"; done`}, + Args: []string{"-c", `stty -echo; while IFS= read -r line; do sleep 0.05; printf "reply:%s\n" "$line"; done`}, Persistent: true, Terminal: true, - ResponseIdleTimeoutMS: 5000, + ResponseIdleTimeoutMS: 150, StartupIdleTimeoutMS: 50, }, }, @@ -188,35 +192,36 @@ func TestCLIExecutePersistentTimeoutCleansSession(t *testing.T) { } defer func() { _ = c.Stop(ctx) }() - firstCtx, firstCancel := context.WithTimeout(ctx, 200*time.Millisecond) + // First execute: cancel before idle timeout fires. + firstCtx, firstCancel := context.WithTimeout(ctx, 100*time.Millisecond) defer firstCancel() sink1 := &testutil.FakeSink{} - err := c.Execute(firstCtx, noderuntime.ExecutionSpec{ + _ = c.Execute(firstCtx, noderuntime.ExecutionSpec{ RunID: "run-1", Model: "slow-echo", Input: map[string]any{"prompt": "first"}, }, sink1) - if err == nil { - t.Fatal("expected context error on first timeout execute") - } - time.Sleep(300 * time.Millisecond) + // Give time for cancel drain to finish (drain uses idleTimeout=150ms). + time.Sleep(400 * time.Millisecond) + + // Second execute on same session — must succeed (session still alive). + execCtx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() sink2 := &testutil.FakeSink{} - err = c.Execute(ctx, noderuntime.ExecutionSpec{ + err := c.Execute(execCtx, noderuntime.ExecutionSpec{ RunID: "run-2", Model: "slow-echo", Input: map[string]any{"prompt": "second"}, }, sink2) - if err == nil { - t.Fatal("expected no-session error on second execute after timeout") + if err != nil { + t.Fatalf("second execute after cancel must succeed (session should be alive): %v", err) } - - for _, e := range sink2.Events() { - if e.Type == noderuntime.EventTypeDelta { - t.Fatalf("unexpected delta event in second run (session contamination): %q", e.Delta) - } + events2 := sink2.Events() + if len(events2) == 0 || events2[len(events2)-1].Type != noderuntime.EventTypeComplete { + t.Fatalf("expected complete event on second run, got %v", events2) } } @@ -278,3 +283,143 @@ func TestCLIExecutePersistentConsecutiveExecutes(t *testing.T) { } } } + +func TestCLIExecutePersistentCreatesIndependentSessionsForSameProfile(t *testing.T) { + testutil.RequirePTYSupport(t) + + cfg := config.CLIConf{ + Enabled: true, + Profiles: map[string]config.CLIProfileConf{ + "multi-echo": { + Command: "sh", + Args: []string{"-c", `stty -echo; while IFS= read -r line; do printf "reply:%s\n" "$line"; done`}, + Persistent: true, + Terminal: true, + ResponseIdleTimeoutMS: 200, + StartupIdleTimeoutMS: 50, + }, + }, + } + c := clipkg.New(cfg, zap.NewNop()) + ctx := context.Background() + + sinkA := &testutil.FakeSink{} + err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-a", + Model: "multi-echo", + SessionID: "session-a", + Input: map[string]any{"prompt": "alpha"}, + }, sinkA) + if err != nil { + t.Fatalf("session-a execute: %v", err) + } + + sinkB := &testutil.FakeSink{} + err = c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-b", + Model: "multi-echo", + SessionID: "session-b", + Input: map[string]any{"prompt": "beta"}, + }, sinkB) + if err != nil { + t.Fatalf("session-b execute: %v", err) + } + + if combined := testutil.CollectDeltas(sinkA.Events()); !strings.Contains(combined, "alpha") { + t.Fatalf("session-a: expected alpha in deltas, got %q", combined) + } + if combined := testutil.CollectDeltas(sinkB.Events()); !strings.Contains(combined, "beta") { + t.Fatalf("session-b: expected beta in deltas, got %q", combined) + } + if combined := testutil.CollectDeltas(sinkA.Events()); strings.Contains(combined, "beta") { + t.Fatalf("session-a contaminated with session-b output: %q", combined) + } +} + +func TestCLIExecutePersistentRequireExistingSessionFailsWhenMissing(t *testing.T) { + testutil.RequireUnixShell(t) + + cfg := config.CLIConf{ + Enabled: true, + Profiles: map[string]config.CLIProfileConf{ + "echo-req": { + Command: "sh", + Args: []string{"-c", `while IFS= read -r line; do printf "reply:%s\n" "$line"; done`}, + Persistent: true, + Terminal: false, + ResponseIdleTimeoutMS: 200, + }, + }, + } + c := clipkg.New(cfg, zap.NewNop()) + ctx := context.Background() + + sink := &testutil.FakeSink{} + err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-1", + Model: "echo-req", + SessionID: "nonexistent", + SessionMode: noderuntime.SessionModeRequireExisting, + Input: map[string]any{"prompt": "hello"}, + }, sink) + if err == nil { + t.Fatal("expected error for require-existing with missing session") + } + if strings.Contains(err.Error(), "start") { + t.Fatalf("should not have started a new process, got: %v", err) + } +} + +func TestCLIExecutePersistentMaintainsHundredLogicalSessions(t *testing.T) { + if testing.Short() { + t.Skip("skipping heavy session test in short mode") + } + testutil.RequireUnixShell(t) + + cfg := config.CLIConf{ + Enabled: true, + Profiles: map[string]config.CLIProfileConf{ + "hundred-echo": { + Command: "sh", + Args: []string{"-c", `while IFS= read -r line; do printf "reply:%s\n" "$line"; done`}, + Persistent: true, + Terminal: false, + ResponseIdleTimeoutMS: 300, + }, + }, + } + c := clipkg.New(cfg, zap.NewNop()) + ctx := context.Background() + + const n = 100 + for i := 0; i < n; i++ { + sessionID := "session-" + string(rune('a'+i%26)) + "-" + string(rune('0'+i/26)) + sink := &testutil.FakeSink{} + err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-first-" + sessionID, + Model: "hundred-echo", + SessionID: sessionID, + Input: map[string]any{"prompt": "ping-" + sessionID}, + }, sink) + if err != nil { + t.Fatalf("session %q first execute: %v", sessionID, err) + } + } + + // Second round — all sessions must still be alive. + for i := 0; i < n; i++ { + sessionID := "session-" + string(rune('a'+i%26)) + "-" + string(rune('0'+i/26)) + sink := &testutil.FakeSink{} + err := c.Execute(ctx, noderuntime.ExecutionSpec{ + RunID: "run-second-" + sessionID, + Model: "hundred-echo", + SessionID: sessionID, + Input: map[string]any{"prompt": "pong-" + sessionID}, + }, sink) + if err != nil { + t.Fatalf("session %q second execute: %v", sessionID, err) + } + } + + _ = c.Stop(ctx) +} diff --git a/apps/node/internal/node/node.go b/apps/node/internal/node/node.go index 6e03dce..b6e112a 100644 --- a/apps/node/internal/node/node.go +++ b/apps/node/internal/node/node.go @@ -5,6 +5,7 @@ package node import ( "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -27,6 +28,7 @@ type Node struct { router runtime.Router registry *adapters.Registry store *store.Store + runs *runManager logger *zap.Logger } @@ -43,6 +45,7 @@ func New( router: router, registry: registry, store: st, + runs: newRunManager(), logger: logger, } } @@ -56,14 +59,17 @@ func (n *Node) OnRunRequest(ctx context.Context, sess *transport.Session, req *i ) rr := runtime.RunRequest{ - RunID: req.GetRunId(), - Adapter: req.GetAdapter(), - Model: req.GetModel(), - Workspace: req.GetWorkspace(), - Policy: structAsMap(req.GetPolicy()), - Input: structAsMap(req.GetInput()), - TimeoutSec: int(req.GetTimeoutSec()), - Metadata: req.GetMetadata(), + RunID: req.GetRunId(), + Adapter: req.GetAdapter(), + Model: req.GetModel(), + SessionID: req.GetSessionId(), + SessionMode: sessionModeFromProto(req.GetSessionMode()), + Background: req.GetBackground(), + Workspace: req.GetWorkspace(), + Policy: structAsMap(req.GetPolicy()), + Input: structAsMap(req.GetInput()), + TimeoutSec: int(req.GetTimeoutSec()), + Metadata: req.GetMetadata(), } printEdgeMessage(os.Stdout, rr.Input) @@ -78,64 +84,115 @@ func (n *Node) OnRunRequest(ctx context.Context, sess *transport.Session, req *i } if err := n.store.InsertRun(ctx, store.RunRecord{ - RunID: spec.RunID, - Adapter: spec.Adapter, - Model: spec.Model, - Status: "running", - CreatedAt: time.Now(), + RunID: spec.RunID, + Adapter: spec.Adapter, + Model: spec.Model, + SessionID: normalizeSessionID(spec.SessionID), + Background: spec.Background, + Status: "running", + CreatedAt: time.Now(), }); err != nil { n.logger.Warn("store: insert run", zap.String("run_id", spec.RunID), zap.Error(err)) } - execCtx := ctx + execCtx, cancel := context.WithCancel(ctx) if spec.TimeoutSec > 0 { - var cancel context.CancelFunc execCtx, cancel = context.WithTimeout(ctx, time.Duration(spec.TimeoutSec)*time.Second) - defer cancel() - sess.RegisterCancel(spec.RunID, cancel) - defer sess.DeregisterCancel(spec.RunID) } - sink := &sessionSink{sess: sess, out: os.Stdout} - execErr := adapter.Execute(execCtx, spec, sink) + h := &runHandle{ + runID: spec.RunID, + adapter: spec.Adapter, + model: spec.Model, + sessionID: normalizeSessionID(spec.SessionID), + cancel: cancel, + done: make(chan struct{}), + } + n.runs.register(h) + sink := &sessionSink{ + sess: sess, + out: os.Stdout, + sessionID: normalizeSessionID(spec.SessionID), + background: spec.Background, + } + + run := func() error { + defer cancel() + defer n.runs.deregister(spec.RunID) + defer close(h.done) + + execErr := adapter.Execute(execCtx, spec, sink) + n.completeRun(spec, execErr) + return execErr + } + + if spec.Background { + go func() { _ = run() }() + return nil + } + return run() +} + +func (n *Node) completeRun(spec runtime.ExecutionSpec, execErr error) { status := "completed" errMsg := "" if execErr != nil { - status = "failed" - errMsg = execErr.Error() - n.logger.Warn("run failed", zap.String("run_id", spec.RunID), zap.Error(execErr)) + if errors.Is(execErr, runtime.ErrRunCancelled) { + status = "cancelled" + } else { + status = "failed" + errMsg = execErr.Error() + } + n.logger.Warn("run ended", zap.String("run_id", spec.RunID), zap.String("status", status), zap.Error(execErr)) } if err := n.store.CompleteRun(context.Background(), spec.RunID, status, errMsg); err != nil { n.logger.Warn("store: complete run", zap.String("run_id", spec.RunID), zap.Error(err)) } - return execErr } -// OnCancel cancels a running execution. -func (n *Node) OnCancel(_ context.Context, sess *transport.Session, req *iop.CancelRequest) error { - n.logger.Info("cancel request", zap.String("run_id", req.GetRunId())) - sess.CancelRun(req.GetRunId()) - return nil +// OnCancel cancels a running execution or terminates an adapter session. +func (n *Node) OnCancel(_ context.Context, _ *transport.Session, req *iop.CancelRequest) error { + n.logger.Info("cancel request", zap.String("run_id", req.GetRunId()), zap.String("action", req.GetAction().String())) + + switch cancelActionFromProto(req.GetAction()) { + case runtime.CancelActionTerminateSession: + adapter, ok := n.registry.Get(req.GetAdapter()) + if !ok { + return fmt.Errorf("node: adapter %q not found", req.GetAdapter()) + } + terminator, ok := adapter.(runtime.SessionTerminator) + if !ok { + return fmt.Errorf("node: adapter %q does not support session termination", req.GetAdapter()) + } + return terminator.TerminateSession(context.Background(), req.GetModel(), normalizeSessionID(req.GetSessionId())) + default: + n.runs.cancelRun(req.GetRunId()) + return nil + } } // sessionSink wraps a transport.Session to implement runtime.EventSink. type sessionSink struct { - sess *transport.Session - out io.Writer - response strings.Builder + sess *transport.Session + out io.Writer + sessionID string + background bool + response strings.Builder } func (s *sessionSink) Emit(_ context.Context, event runtime.RuntimeEvent) error { s.printEvent(event) re := &iop.RunEvent{ - RunId: event.RunID, - Type: string(event.Type), - Delta: event.Delta, - Message: event.Message, - Error: event.Error, - Metadata: event.Metadata, - Timestamp: event.Timestamp.UnixNano(), + RunId: event.RunID, + Type: string(event.Type), + Delta: event.Delta, + Message: event.Message, + Error: event.Error, + Metadata: event.Metadata, + Timestamp: event.Timestamp.UnixNano(), + SessionId: s.sessionID, + Background: s.background, } if event.Usage != nil { re.Usage = &iop.Usage{ @@ -161,6 +218,8 @@ func (s *sessionSink) printEvent(event runtime.RuntimeEvent) { case runtime.EventTypeError: fmt.Fprintf(s.out, "[node-event] error run_id=%s detail=%q\n", event.RunID, event.Error) printTaggedMessage(s.out, "node-message", s.response.String()) + case runtime.EventTypeCancelled: + fmt.Fprintf(s.out, "[node-event] cancelled run_id=%s\n", event.RunID) default: fmt.Fprintf(s.out, "[node-event] %s run_id=%s detail=%q\n", event.Type, event.RunID, event.Message) } @@ -201,3 +260,24 @@ func structAsMap(s *structpb.Struct) map[string]any { } return s.AsMap() } + +func sessionModeFromProto(m iop.RunSessionMode) runtime.SessionMode { + if m == iop.RunSessionMode_RUN_SESSION_MODE_REQUIRE_EXISTING { + return runtime.SessionModeRequireExisting + } + return runtime.SessionModeCreateIfMissing +} + +func cancelActionFromProto(a iop.CancelAction) runtime.CancelAction { + if a == iop.CancelAction_CANCEL_ACTION_TERMINATE_SESSION { + return runtime.CancelActionTerminateSession + } + return runtime.CancelActionCancelRun +} + +func normalizeSessionID(id string) string { + if id == "" { + return runtime.DefaultSessionID + } + return id +} diff --git a/apps/node/internal/node/node_test.go b/apps/node/internal/node/node_test.go index 6339ec7..3a0c594 100644 --- a/apps/node/internal/node/node_test.go +++ b/apps/node/internal/node/node_test.go @@ -6,6 +6,7 @@ import ( "strings" "sync/atomic" "testing" + "time" "go.uber.org/zap" @@ -17,45 +18,95 @@ import ( iop "iop/proto/gen/iop" ) +// --- test doubles --- + type countingAdapter struct { executeCalls int32 } func (a *countingAdapter) Name() string { return "test" } - func (a *countingAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { - return runtime.Capabilities{ - AdapterName: "test", - Models: []string{"v1"}, - MaxConcurrency: 1, - }, nil + return runtime.Capabilities{AdapterName: "test", Models: []string{"v1"}, MaxConcurrency: 1}, nil } - func (a *countingAdapter) Execute(_ context.Context, _ runtime.ExecutionSpec, _ runtime.EventSink) error { atomic.AddInt32(&a.executeCalls, 1) return nil } +// failingAdapter returns a fixed error from Execute. +type failingAdapter struct{ err error } + +func (a *failingAdapter) Name() string { return "failing" } +func (a *failingAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { + return runtime.Capabilities{AdapterName: "failing"}, nil +} +func (a *failingAdapter) Execute(_ context.Context, _ runtime.ExecutionSpec, _ runtime.EventSink) error { + return a.err +} + +// blockingAdapter blocks until its context is cancelled, then returns ErrRunCancelled. +type blockingAdapter struct { + started chan struct{} + done chan struct{} +} + +func newBlockingAdapter() *blockingAdapter { + return &blockingAdapter{started: make(chan struct{}), done: make(chan struct{})} +} + +func (a *blockingAdapter) Name() string { return "blocking" } +func (a *blockingAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { + return runtime.Capabilities{AdapterName: "blocking"}, nil +} +func (a *blockingAdapter) Execute(ctx context.Context, _ runtime.ExecutionSpec, _ runtime.EventSink) error { + close(a.started) + <-ctx.Done() + close(a.done) + return runtime.ErrRunCancelled +} + +// terminatingAdapter implements SessionTerminator. +type terminatingAdapter struct { + terminateCalls int32 + lastModel string + lastSessionID string +} + +func (a *terminatingAdapter) Name() string { return "terminating" } +func (a *terminatingAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { + return runtime.Capabilities{AdapterName: "terminating"}, nil +} +func (a *terminatingAdapter) Execute(_ context.Context, _ runtime.ExecutionSpec, _ runtime.EventSink) error { + return nil +} +func (a *terminatingAdapter) TerminateSession(_ context.Context, model, sessionID string) error { + atomic.AddInt32(&a.terminateCalls, 1) + a.lastModel = model + a.lastSessionID = sessionID + return nil +} + type fixedRouter struct { adapterName string } func (r *fixedRouter) Resolve(_ context.Context, req runtime.RunRequest) (runtime.ExecutionSpec, error) { return runtime.ExecutionSpec{ - RunID: req.RunID, - Adapter: r.adapterName, - Model: req.Model, - Workspace: req.Workspace, - Policy: req.Policy, - Input: req.Input, - TimeoutSec: req.TimeoutSec, - Metadata: req.Metadata, + RunID: req.RunID, + Adapter: r.adapterName, + Model: req.Model, + SessionID: req.SessionID, + SessionMode: req.SessionMode, + Background: req.Background, + Workspace: req.Workspace, + Policy: req.Policy, + Input: req.Input, + TimeoutSec: req.TimeoutSec, + Metadata: req.Metadata, }, nil } -type errorRouter struct { - err error -} +type errorRouter struct{ err error } func (r *errorRouter) Resolve(_ context.Context, _ runtime.RunRequest) (runtime.ExecutionSpec, error) { return runtime.ExecutionSpec{}, r.err @@ -63,20 +114,16 @@ func (r *errorRouter) Resolve(_ context.Context, _ runtime.RunRequest) (runtime. func makeNode(t *testing.T, rtr runtime.Router, reg *adapters.Registry) (*node.Node, *store.Store) { t.Helper() - st, err := store.New(":memory:", zap.NewNop()) if err != nil { t.Fatalf("store: %v", err) } - t.Cleanup(func() { - if err := st.Close(); err != nil { - t.Fatalf("close store: %v", err) - } - }) - + t.Cleanup(func() { _ = st.Close() }) return node.New("test-node", rtr, reg, st, zap.NewNop()), st } +// --- existing tests (updated) --- + func TestOnRunRequest_RouterError(t *testing.T) { reg := adapters.NewRegistry() n, _ := makeNode(t, &errorRouter{err: errors.New("boom")}, reg) @@ -118,7 +165,7 @@ func TestOnRunRequest_Success(t *testing.T) { t.Fatalf("run request: %v", err) } if got := atomic.LoadInt32(&adapter.executeCalls); got != 1 { - t.Fatalf("expected adapter execute call count 1, got %d", got) + t.Fatalf("expected 1 execute call, got %d", got) } run, err := st.GetRun(context.Background(), "run-1") @@ -129,21 +176,165 @@ func TestOnRunRequest_Success(t *testing.T) { t.Fatal("expected persisted run record") } if run.Status != "completed" { - t.Fatalf("expected completed run status, got %q", run.Status) + t.Fatalf("expected completed status, got %q", run.Status) } } -func TestOnCancel_CallsCancelFn(t *testing.T) { +// --- new tests --- + +func TestOnRunRequest_ForegroundAdapterErrorReturned(t *testing.T) { + adapter := &failingAdapter{err: errors.New("adapter boom")} reg := adapters.NewRegistry() - n, _ := makeNode(t, &fixedRouter{adapterName: "test"}, reg) - sess := &transport.Session{} - called := false - sess.RegisterCancel("run-1", func() { called = true }) + reg.Register(adapter) + n, st := makeNode(t, &fixedRouter{adapterName: "failing"}, reg) - if err := n.OnCancel(context.Background(), sess, &iop.CancelRequest{RunId: "run-1"}); err != nil { - t.Fatalf("cancel: %v", err) + err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "run-fail", + Adapter: "failing", + Model: "v1", + }) + if err == nil { + t.Fatal("expected error from OnRunRequest") } - if !called { - t.Fatal("cancel function was not called") + if !strings.Contains(err.Error(), "adapter boom") { + t.Fatalf("expected adapter boom in error, got %v", err) + } + + run, err := st.GetRun(context.Background(), "run-fail") + if err != nil { + t.Fatalf("get run: %v", err) + } + if run == nil { + t.Fatal("expected persisted run record") + } + if run.Status != "failed" { + t.Fatalf("expected failed status, got %q", run.Status) + } + if !strings.Contains(run.Error, "adapter boom") { + t.Fatalf("expected adapter boom in stored error, got %q", run.Error) + } +} + +func TestOnRunRequest_ForegroundCancelReturnedAndStored(t *testing.T) { + adapter := &failingAdapter{err: runtime.ErrRunCancelled} + reg := adapters.NewRegistry() + reg.Register(adapter) + n, st := makeNode(t, &fixedRouter{adapterName: "failing"}, reg) + + err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "run-cancel-fg", + Adapter: "failing", + Model: "v1", + }) + if !errors.Is(err, runtime.ErrRunCancelled) { + t.Fatalf("expected ErrRunCancelled, got %v", err) + } + run, err := st.GetRun(context.Background(), "run-cancel-fg") + if err != nil { + t.Fatalf("get run: %v", err) + } + if run == nil || run.Status != "cancelled" { + t.Fatalf("expected cancelled status, got %+v", run) + } +} + +func TestOnRunRequest_BackgroundReturnsBeforeAdapterCompletes(t *testing.T) { + ba := newBlockingAdapter() + reg := adapters.NewRegistry() + reg.Register(ba) + n, st := makeNode(t, &fixedRouter{adapterName: "blocking"}, reg) + + err := n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "run-bg", + Adapter: "blocking", + Model: "v1", + Background: true, + }) + if err != nil { + t.Fatalf("OnRunRequest: %v", err) + } + + // OnRunRequest should return before adapter finishes. + select { + case <-ba.started: + case <-time.After(2 * time.Second): + t.Fatal("adapter never started") + } + + // Cancel via OnCancel so the blocking adapter finishes. + if err := n.OnCancel(context.Background(), &transport.Session{}, &iop.CancelRequest{RunId: "run-bg"}); err != nil { + t.Fatalf("OnCancel: %v", err) + } + + select { + case <-ba.done: + case <-time.After(2 * time.Second): + t.Fatal("adapter did not finish after cancel") + } + + // Wait briefly for completeRun goroutine to update the store. + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + run, err := st.GetRun(context.Background(), "run-bg") + if err != nil { + t.Fatalf("get run: %v", err) + } + if run != nil && run.Status == "cancelled" { + return + } + time.Sleep(10 * time.Millisecond) + } + run, _ := st.GetRun(context.Background(), "run-bg") + t.Fatalf("expected cancelled status, got %q", run.Status) +} + +func TestOnCancel_CancelsRunViaRunManager(t *testing.T) { + ba := newBlockingAdapter() + reg := adapters.NewRegistry() + reg.Register(ba) + n, _ := makeNode(t, &fixedRouter{adapterName: "blocking"}, reg) + + go func() { + _ = n.OnRunRequest(context.Background(), &transport.Session{}, &iop.RunRequest{ + RunId: "run-cancel", + Adapter: "blocking", + Model: "v1", + }) + }() + + <-ba.started + + if err := n.OnCancel(context.Background(), &transport.Session{}, &iop.CancelRequest{RunId: "run-cancel"}); err != nil { + t.Fatalf("OnCancel: %v", err) + } + + select { + case <-ba.done: + case <-time.After(2 * time.Second): + t.Fatal("adapter did not finish after cancel") + } +} + +func TestOnCancel_TerminatesAdapterSession(t *testing.T) { + ta := &terminatingAdapter{} + reg := adapters.NewRegistry() + reg.Register(ta) + n, _ := makeNode(t, &fixedRouter{adapterName: "terminating"}, reg) + + err := n.OnCancel(context.Background(), &transport.Session{}, &iop.CancelRequest{ + RunId: "", + Adapter: "terminating", + Model: "codex", + SessionId: "session-a", + Action: iop.CancelAction_CANCEL_ACTION_TERMINATE_SESSION, + }) + if err != nil { + t.Fatalf("OnCancel terminate: %v", err) + } + if atomic.LoadInt32(&ta.terminateCalls) != 1 { + t.Fatal("expected TerminateSession to be called once") + } + if ta.lastModel != "codex" || ta.lastSessionID != "session-a" { + t.Fatalf("unexpected terminate args: model=%q session=%q", ta.lastModel, ta.lastSessionID) } } diff --git a/apps/node/internal/node/run_manager.go b/apps/node/internal/node/run_manager.go new file mode 100644 index 0000000..93b1040 --- /dev/null +++ b/apps/node/internal/node/run_manager.go @@ -0,0 +1,47 @@ +package node + +import ( + "context" + "sync" +) + +type runHandle struct { + runID string + adapter string + model string + sessionID string + cancel context.CancelFunc + done chan struct{} +} + +type runManager struct { + mu sync.RWMutex + runs map[string]*runHandle +} + +func newRunManager() *runManager { + return &runManager{runs: make(map[string]*runHandle)} +} + +func (m *runManager) register(h *runHandle) { + m.mu.Lock() + defer m.mu.Unlock() + m.runs[h.runID] = h +} + +func (m *runManager) deregister(runID string) { + m.mu.Lock() + defer m.mu.Unlock() + delete(m.runs, runID) +} + +func (m *runManager) cancelRun(runID string) bool { + m.mu.RLock() + h, ok := m.runs[runID] + m.mu.RUnlock() + if !ok { + return false + } + h.cancel() + return true +} diff --git a/apps/node/internal/router/router.go b/apps/node/internal/router/router.go index bb76cbe..998e155 100644 --- a/apps/node/internal/router/router.go +++ b/apps/node/internal/router/router.go @@ -34,14 +34,17 @@ func (r *defaultRouter) Resolve(_ context.Context, req runtime.RunRequest) (runt } spec := runtime.ExecutionSpec{ - RunID: req.RunID, - Adapter: adapterName, - Model: req.Model, - Workspace: req.Workspace, - Policy: req.Policy, - Input: req.Input, - TimeoutSec: req.TimeoutSec, - Metadata: req.Metadata, + RunID: req.RunID, + Adapter: adapterName, + Model: req.Model, + SessionID: req.SessionID, + SessionMode: req.SessionMode, + Background: req.Background, + Workspace: req.Workspace, + Policy: req.Policy, + Input: req.Input, + TimeoutSec: req.TimeoutSec, + Metadata: req.Metadata, } r.logger.Debug("resolved execution spec", diff --git a/apps/node/internal/router/router_test.go b/apps/node/internal/router/router_test.go new file mode 100644 index 0000000..226dfd4 --- /dev/null +++ b/apps/node/internal/router/router_test.go @@ -0,0 +1,49 @@ +package router + +import ( + "context" + "testing" + + "go.uber.org/zap" + "iop/apps/node/internal/adapters" + "iop/apps/node/internal/runtime" +) + +type stubAdapter struct{ name string } + +func (s *stubAdapter) Name() string { return s.name } +func (s *stubAdapter) Capabilities(_ context.Context) (runtime.Capabilities, error) { + return runtime.Capabilities{AdapterName: s.name}, nil +} +func (s *stubAdapter) Execute(_ context.Context, _ runtime.ExecutionSpec, _ runtime.EventSink) error { + return nil +} + +func TestResolve_PreservesSessionFields(t *testing.T) { + reg := adapters.NewRegistry() + reg.Register(&stubAdapter{name: "cli"}) + r := New(reg, zap.NewNop()) + + req := runtime.RunRequest{ + RunID: "run-1", + Adapter: "cli", + Model: "codex", + SessionID: "session-a", + SessionMode: runtime.SessionModeRequireExisting, + Background: true, + } + + spec, err := r.Resolve(context.Background(), req) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if spec.SessionID != req.SessionID { + t.Errorf("SessionID: got %q want %q", spec.SessionID, req.SessionID) + } + if spec.SessionMode != req.SessionMode { + t.Errorf("SessionMode: got %q want %q", spec.SessionMode, req.SessionMode) + } + if spec.Background != req.Background { + t.Errorf("Background: got %v want %v", spec.Background, req.Background) + } +} diff --git a/apps/node/internal/runtime/types.go b/apps/node/internal/runtime/types.go index 51880b7..0c4c2d8 100644 --- a/apps/node/internal/runtime/types.go +++ b/apps/node/internal/runtime/types.go @@ -3,29 +3,56 @@ package runtime import ( "context" + "errors" "time" ) +// SessionMode controls how a logical worker session is resolved. +type SessionMode string + +const ( + SessionModeCreateIfMissing SessionMode = "create_if_missing" + SessionModeRequireExisting SessionMode = "require_existing" +) + +// CancelAction distinguishes run-cancel from session-terminate. +type CancelAction string + +const ( + CancelActionCancelRun CancelAction = "cancel_run" + CancelActionTerminateSession CancelAction = "terminate_session" +) + +// DefaultSessionID is the session key used when no session_id is provided. +const DefaultSessionID = "default" + +// ErrRunCancelled is returned by adapters when a run is cancelled without terminating the session. +var ErrRunCancelled = errors.New("run cancelled") + // ExecutionSpec is the resolved, policy-applied specification for a single run. type ExecutionSpec struct { - RunID string - Adapter string - Model string - Workspace string - Policy map[string]any - Input map[string]any - TimeoutSec int - Metadata map[string]string + RunID string + Adapter string + Model string + SessionID string + SessionMode SessionMode + Background bool + Workspace string + Policy map[string]any + Input map[string]any + TimeoutSec int + Metadata map[string]string } // EventType classifies a RuntimeEvent. type EventType string const ( - EventTypeStart EventType = "start" - EventTypeDelta EventType = "delta" - EventTypeComplete EventType = "complete" - EventTypeError EventType = "error" + EventTypeStart EventType = "start" + EventTypeDelta EventType = "delta" + EventTypeComplete EventType = "complete" + EventTypeError EventType = "error" + EventTypeCancelled EventType = "cancelled" ) // RuntimeEvent is a streaming execution event emitted by an Adapter. @@ -55,14 +82,23 @@ type Capabilities struct { // RunRequest is the node-domain representation of an incoming run request. type RunRequest struct { - RunID string - Adapter string - Model string - Workspace string - Policy map[string]any - Input map[string]any - TimeoutSec int - Metadata map[string]string + RunID string + Adapter string + Model string + SessionID string + SessionMode SessionMode + Background bool + Workspace string + Policy map[string]any + Input map[string]any + TimeoutSec int + Metadata map[string]string +} + +// SessionTerminator is an optional interface Adapters may implement to support +// explicit session lifecycle management separate from run cancellation. +type SessionTerminator interface { + TerminateSession(ctx context.Context, model, sessionID string) error } // EventSink receives RuntimeEvents emitted during execution. diff --git a/apps/node/internal/store/store.go b/apps/node/internal/store/store.go index 484c757..ccbede2 100644 --- a/apps/node/internal/store/store.go +++ b/apps/node/internal/store/store.go @@ -16,6 +16,8 @@ CREATE TABLE IF NOT EXISTS runs ( run_id TEXT PRIMARY KEY, adapter TEXT NOT NULL, model TEXT NOT NULL, + session_id TEXT NOT NULL DEFAULT 'default', + background INTEGER NOT NULL DEFAULT 0, status TEXT NOT NULL DEFAULT 'running', created_at DATETIME NOT NULL, completed_at DATETIME, @@ -28,6 +30,8 @@ type RunRecord struct { RunID string Adapter string Model string + SessionID string + Background bool Status string CreatedAt time.Time CompletedAt *time.Time @@ -48,12 +52,59 @@ func New(dsn string, logger *zap.Logger) (*Store, error) { } db.SetMaxOpenConns(1) // SQLite is single-writer if _, err := db.Exec(schema); err != nil { + _ = db.Close() return nil, fmt.Errorf("store: migrate: %w", err) } + if err := migrateRunsTable(db); err != nil { + _ = db.Close() + return nil, fmt.Errorf("store: migrate runs: %w", err) + } logger.Info("store ready", zap.String("dsn", dsn)) return &Store{db: db, logger: logger}, nil } +func migrateRunsTable(db *sql.DB) error { + cols, err := tableColumns(db, "runs") + if err != nil { + return err + } + if !cols["session_id"] { + if _, err := db.Exec(`ALTER TABLE runs ADD COLUMN session_id TEXT NOT NULL DEFAULT 'default'`); err != nil { + return err + } + } + if !cols["background"] { + if _, err := db.Exec(`ALTER TABLE runs ADD COLUMN background INTEGER NOT NULL DEFAULT 0`); err != nil { + return err + } + } + return nil +} + +func tableColumns(db *sql.DB, table string) (map[string]bool, error) { + rows, err := db.Query(fmt.Sprintf("PRAGMA table_info(%s)", table)) + if err != nil { + return nil, err + } + defer rows.Close() + cols := make(map[string]bool) + for rows.Next() { + var ( + cid int + name string + ctype string + notnull int + dfltValue sql.NullString + pk int + ) + if err := rows.Scan(&cid, &name, &ctype, ¬null, &dfltValue, &pk); err != nil { + return nil, err + } + cols[name] = true + } + return cols, rows.Err() +} + // Close closes the underlying database connection. func (s *Store) Close() error { return s.db.Close() @@ -61,14 +112,22 @@ func (s *Store) Close() error { // InsertRun records a new run. func (s *Store) InsertRun(ctx context.Context, r RunRecord) error { + sessionID := r.SessionID + if sessionID == "" { + sessionID = "default" + } + bg := 0 + if r.Background { + bg = 1 + } _, err := s.db.ExecContext(ctx, - `INSERT INTO runs (run_id, adapter, model, status, created_at) VALUES (?,?,?,?,?)`, - r.RunID, r.Adapter, r.Model, r.Status, r.CreatedAt.UTC(), + `INSERT INTO runs (run_id, adapter, model, session_id, background, status, created_at) VALUES (?,?,?,?,?,?,?)`, + r.RunID, r.Adapter, r.Model, sessionID, bg, r.Status, r.CreatedAt.UTC(), ) return err } -// CompleteRun marks a run as completed or failed. +// CompleteRun marks a run as completed, failed, or cancelled. func (s *Store) CompleteRun(ctx context.Context, runID, status, errMsg string) error { _, err := s.db.ExecContext(ctx, `UPDATE runs SET status=?, completed_at=?, error=? WHERE run_id=?`, @@ -80,18 +139,20 @@ func (s *Store) CompleteRun(ctx context.Context, runID, status, errMsg string) e // GetRun retrieves a run record by ID. func (s *Store) GetRun(ctx context.Context, runID string) (*RunRecord, error) { row := s.db.QueryRowContext(ctx, - `SELECT run_id, adapter, model, status, created_at, completed_at, error FROM runs WHERE run_id=?`, + `SELECT run_id, adapter, model, session_id, background, status, created_at, completed_at, error FROM runs WHERE run_id=?`, runID, ) var r RunRecord + var bg int var completedAt sql.NullTime var errMsg sql.NullString - if err := row.Scan(&r.RunID, &r.Adapter, &r.Model, &r.Status, &r.CreatedAt, &completedAt, &errMsg); err != nil { + if err := row.Scan(&r.RunID, &r.Adapter, &r.Model, &r.SessionID, &bg, &r.Status, &r.CreatedAt, &completedAt, &errMsg); err != nil { if err == sql.ErrNoRows { return nil, nil } return nil, err } + r.Background = bg != 0 if completedAt.Valid { r.CompletedAt = &completedAt.Time } diff --git a/apps/node/internal/store/store_test.go b/apps/node/internal/store/store_test.go new file mode 100644 index 0000000..74c0efb --- /dev/null +++ b/apps/node/internal/store/store_test.go @@ -0,0 +1,181 @@ +package store_test + +import ( + "context" + "database/sql" + "path/filepath" + "testing" + "time" + + _ "modernc.org/sqlite" + + "go.uber.org/zap" + + "iop/apps/node/internal/store" +) + +const oldRunsSchema = ` +CREATE TABLE runs ( + run_id TEXT PRIMARY KEY, + adapter TEXT NOT NULL, + model TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'running', + created_at DATETIME NOT NULL, + completed_at DATETIME, + error TEXT +); +` + +func seedOldSchemaDB(t *testing.T) string { + t.Helper() + dir := t.TempDir() + dsn := filepath.Join(dir, "old.db") + db, err := sql.Open("sqlite", dsn) + if err != nil { + t.Fatalf("open: %v", err) + } + if _, err := db.Exec(oldRunsSchema); err != nil { + t.Fatalf("seed schema: %v", err) + } + if err := db.Close(); err != nil { + t.Fatalf("close: %v", err) + } + return dsn +} + +func newTestStore(t *testing.T) *store.Store { + t.Helper() + s, err := store.New(":memory:", zap.NewNop()) + if err != nil { + t.Fatalf("store.New: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + return s +} + +func TestStore_RunSessionFields(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + + r := store.RunRecord{ + RunID: "run-1", + Adapter: "cli", + Model: "codex", + SessionID: "session-a", + Background: true, + Status: "running", + CreatedAt: time.Now(), + } + if err := s.InsertRun(ctx, r); err != nil { + t.Fatalf("InsertRun: %v", err) + } + + got, err := s.GetRun(ctx, "run-1") + if err != nil { + t.Fatalf("GetRun: %v", err) + } + if got == nil { + t.Fatal("expected run record") + } + if got.SessionID != "session-a" { + t.Errorf("SessionID: got %q want %q", got.SessionID, "session-a") + } + if !got.Background { + t.Error("Background: expected true") + } + if got.Status != "running" { + t.Errorf("Status: got %q want %q", got.Status, "running") + } + + if err := s.CompleteRun(ctx, "run-1", "cancelled", ""); err != nil { + t.Fatalf("CompleteRun: %v", err) + } + got, err = s.GetRun(ctx, "run-1") + if err != nil { + t.Fatalf("GetRun after complete: %v", err) + } + if got.Status != "cancelled" { + t.Errorf("Status after cancel: got %q want %q", got.Status, "cancelled") + } +} + +func TestStore_DefaultSessionID(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + + r := store.RunRecord{ + RunID: "run-2", + Adapter: "cli", + Model: "codex", + SessionID: "", // empty → should default to "default" + Status: "running", + CreatedAt: time.Now(), + } + if err := s.InsertRun(ctx, r); err != nil { + t.Fatalf("InsertRun: %v", err) + } + + got, err := s.GetRun(ctx, "run-2") + if err != nil { + t.Fatalf("GetRun: %v", err) + } + if got.SessionID != "default" { + t.Errorf("SessionID: got %q want %q", got.SessionID, "default") + } +} + +func TestStore_MigratesExistingRunsTable(t *testing.T) { + dsn := seedOldSchemaDB(t) + + s, err := store.New(dsn, zap.NewNop()) + if err != nil { + t.Fatalf("store.New: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + + ctx := context.Background() + r := store.RunRecord{ + RunID: "run-mig", + Adapter: "cli", + Model: "codex", + SessionID: "session-a", + Background: true, + Status: "running", + CreatedAt: time.Now(), + } + if err := s.InsertRun(ctx, r); err != nil { + t.Fatalf("InsertRun: %v", err) + } + + got, err := s.GetRun(ctx, "run-mig") + if err != nil { + t.Fatalf("GetRun: %v", err) + } + if got == nil { + t.Fatal("expected run record") + } + if got.SessionID != "session-a" { + t.Errorf("SessionID: got %q want %q", got.SessionID, "session-a") + } + if !got.Background { + t.Error("Background: expected true") + } +} + +func TestStore_MigrationIsIdempotent(t *testing.T) { + dsn := seedOldSchemaDB(t) + + s1, err := store.New(dsn, zap.NewNop()) + if err != nil { + t.Fatalf("store.New first: %v", err) + } + if err := s1.Close(); err != nil { + t.Fatalf("close first: %v", err) + } + + s2, err := store.New(dsn, zap.NewNop()) + if err != nil { + t.Fatalf("store.New second: %v", err) + } + t.Cleanup(func() { _ = s2.Close() }) +} diff --git a/apps/node/internal/transport/parser_test.go b/apps/node/internal/transport/parser_test.go index 2f277ca..fc06256 100644 --- a/apps/node/internal/transport/parser_test.go +++ b/apps/node/internal/transport/parser_test.go @@ -11,7 +11,14 @@ import ( func TestNodeParserMap_RunRequest(t *testing.T) { parsers := nodeParserMap() - original := &iop.RunRequest{RunId: "run-1", Adapter: "mock", Model: "v1"} + original := &iop.RunRequest{ + RunId: "run-1", + Adapter: "mock", + Model: "v1", + SessionId: "session-a", + Background: true, + SessionMode: iop.RunSessionMode_RUN_SESSION_MODE_REQUIRE_EXISTING, + } payload, err := proto.Marshal(original) if err != nil { t.Fatalf("marshal: %v", err) @@ -30,14 +37,23 @@ func TestNodeParserMap_RunRequest(t *testing.T) { got := parsed.(*iop.RunRequest) if got.GetRunId() != original.GetRunId() || got.GetAdapter() != original.GetAdapter() || - got.GetModel() != original.GetModel() { + got.GetModel() != original.GetModel() || + got.GetSessionId() != original.GetSessionId() || + got.GetBackground() != original.GetBackground() || + got.GetSessionMode() != original.GetSessionMode() { t.Fatalf("unexpected run request: %+v", got) } } func TestNodeParserMap_CancelRequest(t *testing.T) { parsers := nodeParserMap() - original := &iop.CancelRequest{RunId: "run-1"} + original := &iop.CancelRequest{ + RunId: "run-1", + Adapter: "cli", + Model: "codex", + SessionId: "session-a", + Action: iop.CancelAction_CANCEL_ACTION_TERMINATE_SESSION, + } payload, err := proto.Marshal(original) if err != nil { t.Fatalf("marshal: %v", err) @@ -54,7 +70,11 @@ func TestNodeParserMap_CancelRequest(t *testing.T) { t.Fatalf("parse: %v", err) } got := parsed.(*iop.CancelRequest) - if got.GetRunId() != original.GetRunId() { + if got.GetRunId() != original.GetRunId() || + got.GetAdapter() != original.GetAdapter() || + got.GetModel() != original.GetModel() || + got.GetSessionId() != original.GetSessionId() || + got.GetAction() != original.GetAction() { t.Fatalf("unexpected cancel request: %+v", got) } } diff --git a/apps/node/internal/transport/session.go b/apps/node/internal/transport/session.go index af36819..12c3c12 100644 --- a/apps/node/internal/transport/session.go +++ b/apps/node/internal/transport/session.go @@ -19,11 +19,10 @@ type Handler interface { // Session represents the node's persistent connection to edge. type Session struct { - client *toki.TcpClient - logger *zap.Logger - mu sync.RWMutex - handler Handler - cancelFns sync.Map + client *toki.TcpClient + logger *zap.Logger + mu sync.RWMutex + handler Handler } func newSession(client *toki.TcpClient, logger *zap.Logger) *Session { @@ -85,20 +84,3 @@ func (s *Session) IsAlive() bool { return s.client.IsAlive() } // Close terminates the connection to edge. func (s *Session) Close() error { return s.client.Close() } - -// RegisterCancel stores a cancel function for a running execution. -func (s *Session) RegisterCancel(runID string, cancel context.CancelFunc) { - s.cancelFns.Store(runID, cancel) -} - -// DeregisterCancel removes a run's cancel function after completion. -func (s *Session) DeregisterCancel(runID string) { - s.cancelFns.Delete(runID) -} - -// CancelRun invokes the cancel function for a run if one is registered. -func (s *Session) CancelRun(runID string) { - if v, ok := s.cancelFns.Load(runID); ok { - v.(context.CancelFunc)() - } -} diff --git a/apps/node/internal/transport/session_test.go b/apps/node/internal/transport/session_test.go index 3733372..2296038 100644 --- a/apps/node/internal/transport/session_test.go +++ b/apps/node/internal/transport/session_test.go @@ -1,58 +1,33 @@ package transport_test import ( - "fmt" + "context" "sync" "testing" "iop/apps/node/internal/transport" + iop "iop/proto/gen/iop" ) -func TestSession_RegisterCancel_CancelRun(t *testing.T) { - sess := &transport.Session{} - called := false +type noopHandler struct{} - sess.RegisterCancel("run-1", func() { called = true }) - sess.CancelRun("run-1") - - if !called { - t.Fatal("cancel function was not called") - } +func (h *noopHandler) OnRunRequest(_ context.Context, _ *transport.Session, _ *iop.RunRequest) error { + return nil +} +func (h *noopHandler) OnCancel(_ context.Context, _ *transport.Session, _ *iop.CancelRequest) error { + return nil } -func TestSession_DeregisterCancel(t *testing.T) { - sess := &transport.Session{} - called := false - - sess.RegisterCancel("run-1", func() { called = true }) - sess.DeregisterCancel("run-1") - sess.CancelRun("run-1") - - if called { - t.Fatal("cancel function should not have been called after deregister") - } -} - -func TestSession_CancelRun_UnknownID(t *testing.T) { - sess := &transport.Session{} - - sess.CancelRun("missing-run") -} - -func TestSession_ConcurrentRegisterCancel(t *testing.T) { - sess := &transport.Session{} +func TestSession_SetHandler_ConcurrentSafe(t *testing.T) { + var s transport.Session var wg sync.WaitGroup - - for i := 0; i < 100; i++ { - id := fmt.Sprintf("run-%d", i) + h := &noopHandler{} + for i := 0; i < 50; i++ { wg.Add(1) - go func(runID string) { + go func() { defer wg.Done() - sess.RegisterCancel(runID, func() {}) - sess.CancelRun(runID) - sess.DeregisterCancel(runID) - }(id) + s.SetHandler(h) + }() } - wg.Wait() } diff --git a/configs/edge.yaml b/configs/edge.yaml index 08499e7..9e8579b 100644 --- a/configs/edge.yaml +++ b/configs/edge.yaml @@ -14,6 +14,8 @@ metrics: console: adapter: "cli" model: "codex" + session_id: "default" + background: false timeout_sec: 120 nodes: @@ -55,7 +57,9 @@ nodes: - "never" - "--skip-git-repo-check" env: [] - persistent: false + # persistent: true enables logical worker session reuse. + # Set to false to use one-shot mode (new process per request). + persistent: true terminal: false runtime: concurrency: 4 diff --git a/packages/config/config.go b/packages/config/config.go index 1f999ca..7b9653e 100644 --- a/packages/config/config.go +++ b/packages/config/config.go @@ -39,6 +39,8 @@ type EdgeServerConf struct { type EdgeConsoleConf struct { Adapter string `mapstructure:"adapter" yaml:"adapter"` Model string `mapstructure:"model" yaml:"model"` + SessionID string `mapstructure:"session_id" yaml:"session_id"` + Background bool `mapstructure:"background" yaml:"background"` TimeoutSec int `mapstructure:"timeout_sec" yaml:"timeout_sec"` } @@ -144,5 +146,7 @@ func setEdgeDefaults(v *viper.Viper) { v.SetDefault("tls.enabled", false) v.SetDefault("console.adapter", "cli") v.SetDefault("console.model", "codex") + v.SetDefault("console.session_id", "default") + v.SetDefault("console.background", false) v.SetDefault("console.timeout_sec", 120) } diff --git a/packages/config/config_test.go b/packages/config/config_test.go index 29311f8..9dfa58a 100644 --- a/packages/config/config_test.go +++ b/packages/config/config_test.go @@ -30,6 +30,43 @@ func TestLoadEdge_ConsoleTimeoutDefault(t *testing.T) { } } +func TestLoadEdge_ConsoleSessionDefaults(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "edge.yaml") + if err := os.WriteFile(f, []byte("server:\n listen: \"0.0.0.0:9090\"\n"), 0o600); err != nil { + t.Fatalf("write yaml: %v", err) + } + cfg, err := config.LoadEdge(f) + if err != nil { + t.Fatalf("load: %v", err) + } + if cfg.Console.SessionID != "default" { + t.Fatalf("expected default session_id=%q, got %q", "default", cfg.Console.SessionID) + } + if cfg.Console.Background { + t.Fatal("expected default background=false") + } +} + +func TestLoadEdge_ConsoleSessionOverride(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "edge.yaml") + yaml := "server:\n listen: \"0.0.0.0:9090\"\nconsole:\n session_id: \"worker-1\"\n background: true\n" + if err := os.WriteFile(f, []byte(yaml), 0o600); err != nil { + t.Fatalf("write yaml: %v", err) + } + cfg, err := config.LoadEdge(f) + if err != nil { + t.Fatalf("load: %v", err) + } + if cfg.Console.SessionID != "worker-1" { + t.Fatalf("expected session_id=%q, got %q", "worker-1", cfg.Console.SessionID) + } + if !cfg.Console.Background { + t.Fatal("expected background=true") + } +} + func TestLoadEdge_ConsoleTimeoutOverride(t *testing.T) { dir := t.TempDir() f := filepath.Join(dir, "edge.yaml") diff --git a/proto/gen/iop/runtime.pb.go b/proto/gen/iop/runtime.pb.go index 172e7cf..8f80589 100644 --- a/proto/gen/iop/runtime.pb.go +++ b/proto/gen/iop/runtime.pb.go @@ -22,6 +22,104 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) +type RunSessionMode int32 + +const ( + RunSessionMode_RUN_SESSION_MODE_UNSPECIFIED RunSessionMode = 0 // default: create if missing + RunSessionMode_RUN_SESSION_MODE_CREATE_IF_MISSING RunSessionMode = 1 + RunSessionMode_RUN_SESSION_MODE_REQUIRE_EXISTING RunSessionMode = 2 +) + +// Enum value maps for RunSessionMode. +var ( + RunSessionMode_name = map[int32]string{ + 0: "RUN_SESSION_MODE_UNSPECIFIED", + 1: "RUN_SESSION_MODE_CREATE_IF_MISSING", + 2: "RUN_SESSION_MODE_REQUIRE_EXISTING", + } + RunSessionMode_value = map[string]int32{ + "RUN_SESSION_MODE_UNSPECIFIED": 0, + "RUN_SESSION_MODE_CREATE_IF_MISSING": 1, + "RUN_SESSION_MODE_REQUIRE_EXISTING": 2, + } +) + +func (x RunSessionMode) Enum() *RunSessionMode { + p := new(RunSessionMode) + *p = x + return p +} + +func (x RunSessionMode) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (RunSessionMode) Descriptor() protoreflect.EnumDescriptor { + return file_proto_iop_runtime_proto_enumTypes[0].Descriptor() +} + +func (RunSessionMode) Type() protoreflect.EnumType { + return &file_proto_iop_runtime_proto_enumTypes[0] +} + +func (x RunSessionMode) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use RunSessionMode.Descriptor instead. +func (RunSessionMode) EnumDescriptor() ([]byte, []int) { + return file_proto_iop_runtime_proto_rawDescGZIP(), []int{0} +} + +type CancelAction int32 + +const ( + CancelAction_CANCEL_ACTION_UNSPECIFIED CancelAction = 0 // default: cancel run only + CancelAction_CANCEL_ACTION_CANCEL_RUN CancelAction = 1 + CancelAction_CANCEL_ACTION_TERMINATE_SESSION CancelAction = 2 +) + +// Enum value maps for CancelAction. +var ( + CancelAction_name = map[int32]string{ + 0: "CANCEL_ACTION_UNSPECIFIED", + 1: "CANCEL_ACTION_CANCEL_RUN", + 2: "CANCEL_ACTION_TERMINATE_SESSION", + } + CancelAction_value = map[string]int32{ + "CANCEL_ACTION_UNSPECIFIED": 0, + "CANCEL_ACTION_CANCEL_RUN": 1, + "CANCEL_ACTION_TERMINATE_SESSION": 2, + } +) + +func (x CancelAction) Enum() *CancelAction { + p := new(CancelAction) + *p = x + return p +} + +func (x CancelAction) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (CancelAction) Descriptor() protoreflect.EnumDescriptor { + return file_proto_iop_runtime_proto_enumTypes[1].Descriptor() +} + +func (CancelAction) Type() protoreflect.EnumType { + return &file_proto_iop_runtime_proto_enumTypes[1] +} + +func (x CancelAction) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use CancelAction.Descriptor instead. +func (CancelAction) EnumDescriptor() ([]byte, []int) { + return file_proto_iop_runtime_proto_rawDescGZIP(), []int{1} +} + // RunRequest initiates a model execution. type RunRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -33,6 +131,9 @@ type RunRequest struct { Input *structpb.Struct `protobuf:"bytes,6,opt,name=input,proto3" json:"input,omitempty"` TimeoutSec int32 `protobuf:"varint,7,opt,name=timeout_sec,json=timeoutSec,proto3" json:"timeout_sec,omitempty"` Metadata map[string]string `protobuf:"bytes,8,rep,name=metadata,proto3" json:"metadata,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` + SessionId string `protobuf:"bytes,9,opt,name=session_id,json=sessionId,proto3" json:"session_id,omitempty"` + SessionMode RunSessionMode `protobuf:"varint,10,opt,name=session_mode,json=sessionMode,proto3,enum=iop.RunSessionMode" json:"session_mode,omitempty"` + Background bool `protobuf:"varint,11,opt,name=background,proto3" json:"background,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -123,17 +224,40 @@ func (x *RunRequest) GetMetadata() map[string]string { return nil } +func (x *RunRequest) GetSessionId() string { + if x != nil { + return x.SessionId + } + return "" +} + +func (x *RunRequest) GetSessionMode() RunSessionMode { + if x != nil { + return x.SessionMode + } + return RunSessionMode_RUN_SESSION_MODE_UNSPECIFIED +} + +func (x *RunRequest) GetBackground() bool { + if x != nil { + return x.Background + } + return false +} + // RunEvent is a streaming execution event. type RunEvent struct { state protoimpl.MessageState `protogen:"open.v1"` RunId string `protobuf:"bytes,1,opt,name=run_id,json=runId,proto3" json:"run_id,omitempty"` - Type string `protobuf:"bytes,2,opt,name=type,proto3" json:"type,omitempty"` // start | delta | complete | error + Type string `protobuf:"bytes,2,opt,name=type,proto3" json:"type,omitempty"` // start | delta | complete | error | cancelled Delta string `protobuf:"bytes,3,opt,name=delta,proto3" json:"delta,omitempty"` Message string `protobuf:"bytes,4,opt,name=message,proto3" json:"message,omitempty"` Error string `protobuf:"bytes,5,opt,name=error,proto3" json:"error,omitempty"` Usage *Usage `protobuf:"bytes,6,opt,name=usage,proto3" json:"usage,omitempty"` Metadata map[string]string `protobuf:"bytes,7,rep,name=metadata,proto3" json:"metadata,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` Timestamp int64 `protobuf:"varint,8,opt,name=timestamp,proto3" json:"timestamp,omitempty"` // unix nano + SessionId string `protobuf:"bytes,9,opt,name=session_id,json=sessionId,proto3" json:"session_id,omitempty"` + Background bool `protobuf:"varint,10,opt,name=background,proto3" json:"background,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -224,6 +348,20 @@ func (x *RunEvent) GetTimestamp() int64 { return 0 } +func (x *RunEvent) GetSessionId() string { + if x != nil { + return x.SessionId + } + return "" +} + +func (x *RunEvent) GetBackground() bool { + if x != nil { + return x.Background + } + return false +} + type Usage struct { state protoimpl.MessageState `protogen:"open.v1"` InputTokens int32 `protobuf:"varint,1,opt,name=input_tokens,json=inputTokens,proto3" json:"input_tokens,omitempty"` @@ -325,6 +463,10 @@ func (x *Heartbeat) GetTimestamp() int64 { type CancelRequest struct { state protoimpl.MessageState `protogen:"open.v1"` RunId string `protobuf:"bytes,1,opt,name=run_id,json=runId,proto3" json:"run_id,omitempty"` + Adapter string `protobuf:"bytes,2,opt,name=adapter,proto3" json:"adapter,omitempty"` + Model string `protobuf:"bytes,3,opt,name=model,proto3" json:"model,omitempty"` + SessionId string `protobuf:"bytes,4,opt,name=session_id,json=sessionId,proto3" json:"session_id,omitempty"` + Action CancelAction `protobuf:"varint,5,opt,name=action,proto3,enum=iop.CancelAction" json:"action,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -366,6 +508,34 @@ func (x *CancelRequest) GetRunId() string { return "" } +func (x *CancelRequest) GetAdapter() string { + if x != nil { + return x.Adapter + } + return "" +} + +func (x *CancelRequest) GetModel() string { + if x != nil { + return x.Model + } + return "" +} + +func (x *CancelRequest) GetSessionId() string { + if x != nil { + return x.SessionId + } + return "" +} + +func (x *CancelRequest) GetAction() CancelAction { + if x != nil { + return x.Action + } + return CancelAction_CANCEL_ACTION_UNSPECIFIED +} + // Error is returned when a request fails at the transport layer. type Error struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -712,7 +882,7 @@ var File_proto_iop_runtime_proto protoreflect.FileDescriptor const file_proto_iop_runtime_proto_rawDesc = "" + "\n" + - "\x17proto/iop/runtime.proto\x12\x03iop\x1a\x1cgoogle/protobuf/struct.proto\"\xea\x02\n" + + "\x17proto/iop/runtime.proto\x12\x03iop\x1a\x1cgoogle/protobuf/struct.proto\"\xe1\x03\n" + "\n" + "RunRequest\x12\x15\n" + "\x06run_id\x18\x01 \x01(\tR\x05runId\x12\x18\n" + @@ -723,10 +893,17 @@ const file_proto_iop_runtime_proto_rawDesc = "" + "\x05input\x18\x06 \x01(\v2\x17.google.protobuf.StructR\x05input\x12\x1f\n" + "\vtimeout_sec\x18\a \x01(\x05R\n" + "timeoutSec\x129\n" + - "\bmetadata\x18\b \x03(\v2\x1d.iop.RunRequest.MetadataEntryR\bmetadata\x1a;\n" + + "\bmetadata\x18\b \x03(\v2\x1d.iop.RunRequest.MetadataEntryR\bmetadata\x12\x1d\n" + + "\n" + + "session_id\x18\t \x01(\tR\tsessionId\x126\n" + + "\fsession_mode\x18\n" + + " \x01(\x0e2\x13.iop.RunSessionModeR\vsessionMode\x12\x1e\n" + + "\n" + + "background\x18\v \x01(\bR\n" + + "background\x1a;\n" + "\rMetadataEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xb1\x02\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xf0\x02\n" + "\bRunEvent\x12\x15\n" + "\x06run_id\x18\x01 \x01(\tR\x05runId\x12\x12\n" + "\x04type\x18\x02 \x01(\tR\x04type\x12\x14\n" + @@ -736,7 +913,13 @@ const file_proto_iop_runtime_proto_rawDesc = "" + "\x05usage\x18\x06 \x01(\v2\n" + ".iop.UsageR\x05usage\x127\n" + "\bmetadata\x18\a \x03(\v2\x1b.iop.RunEvent.MetadataEntryR\bmetadata\x12\x1c\n" + - "\ttimestamp\x18\b \x01(\x03R\ttimestamp\x1a;\n" + + "\ttimestamp\x18\b \x01(\x03R\ttimestamp\x12\x1d\n" + + "\n" + + "session_id\x18\t \x01(\tR\tsessionId\x12\x1e\n" + + "\n" + + "background\x18\n" + + " \x01(\bR\n" + + "background\x1a;\n" + "\rMetadataEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"O\n" + @@ -744,9 +927,14 @@ const file_proto_iop_runtime_proto_rawDesc = "" + "\finput_tokens\x18\x01 \x01(\x05R\vinputTokens\x12#\n" + "\routput_tokens\x18\x02 \x01(\x05R\foutputTokens\")\n" + "\tHeartbeat\x12\x1c\n" + - "\ttimestamp\x18\x01 \x01(\x03R\ttimestamp\"&\n" + + "\ttimestamp\x18\x01 \x01(\x03R\ttimestamp\"\xa0\x01\n" + "\rCancelRequest\x12\x15\n" + - "\x06run_id\x18\x01 \x01(\tR\x05runId\"5\n" + + "\x06run_id\x18\x01 \x01(\tR\x05runId\x12\x18\n" + + "\aadapter\x18\x02 \x01(\tR\aadapter\x12\x14\n" + + "\x05model\x18\x03 \x01(\tR\x05model\x12\x1d\n" + + "\n" + + "session_id\x18\x04 \x01(\tR\tsessionId\x12)\n" + + "\x06action\x18\x05 \x01(\x0e2\x11.iop.CancelActionR\x06action\"5\n" + "\x05Error\x12\x12\n" + "\x04code\x18\x01 \x01(\tR\x04code\x12\x18\n" + "\amessage\x18\x02 \x01(\tR\amessage\"'\n" + @@ -767,7 +955,15 @@ const file_proto_iop_runtime_proto_rawDesc = "" + "\bsettings\x18\x03 \x01(\v2\x17.google.protobuf.StructR\bsettings\"\\\n" + "\x11NodeRuntimeConfig\x12 \n" + "\vconcurrency\x18\x01 \x01(\x05R\vconcurrency\x12%\n" + - "\x0eworkspace_root\x18\x02 \x01(\tR\rworkspaceRootB\x13Z\x11iop/proto/gen/iopb\x06proto3" + "\x0eworkspace_root\x18\x02 \x01(\tR\rworkspaceRoot*\x81\x01\n" + + "\x0eRunSessionMode\x12 \n" + + "\x1cRUN_SESSION_MODE_UNSPECIFIED\x10\x00\x12&\n" + + "\"RUN_SESSION_MODE_CREATE_IF_MISSING\x10\x01\x12%\n" + + "!RUN_SESSION_MODE_REQUIRE_EXISTING\x10\x02*p\n" + + "\fCancelAction\x12\x1d\n" + + "\x19CANCEL_ACTION_UNSPECIFIED\x10\x00\x12\x1c\n" + + "\x18CANCEL_ACTION_CANCEL_RUN\x10\x01\x12#\n" + + "\x1fCANCEL_ACTION_TERMINATE_SESSION\x10\x02B\x13Z\x11iop/proto/gen/iopb\x06proto3" var ( file_proto_iop_runtime_proto_rawDescOnce sync.Once @@ -781,38 +977,43 @@ func file_proto_iop_runtime_proto_rawDescGZIP() []byte { return file_proto_iop_runtime_proto_rawDescData } +var file_proto_iop_runtime_proto_enumTypes = make([]protoimpl.EnumInfo, 2) var file_proto_iop_runtime_proto_msgTypes = make([]protoimpl.MessageInfo, 13) var file_proto_iop_runtime_proto_goTypes = []any{ - (*RunRequest)(nil), // 0: iop.RunRequest - (*RunEvent)(nil), // 1: iop.RunEvent - (*Usage)(nil), // 2: iop.Usage - (*Heartbeat)(nil), // 3: iop.Heartbeat - (*CancelRequest)(nil), // 4: iop.CancelRequest - (*Error)(nil), // 5: iop.Error - (*RegisterRequest)(nil), // 6: iop.RegisterRequest - (*RegisterResponse)(nil), // 7: iop.RegisterResponse - (*NodeConfigPayload)(nil), // 8: iop.NodeConfigPayload - (*AdapterConfig)(nil), // 9: iop.AdapterConfig - (*NodeRuntimeConfig)(nil), // 10: iop.NodeRuntimeConfig - nil, // 11: iop.RunRequest.MetadataEntry - nil, // 12: iop.RunEvent.MetadataEntry - (*structpb.Struct)(nil), // 13: google.protobuf.Struct + (RunSessionMode)(0), // 0: iop.RunSessionMode + (CancelAction)(0), // 1: iop.CancelAction + (*RunRequest)(nil), // 2: iop.RunRequest + (*RunEvent)(nil), // 3: iop.RunEvent + (*Usage)(nil), // 4: iop.Usage + (*Heartbeat)(nil), // 5: iop.Heartbeat + (*CancelRequest)(nil), // 6: iop.CancelRequest + (*Error)(nil), // 7: iop.Error + (*RegisterRequest)(nil), // 8: iop.RegisterRequest + (*RegisterResponse)(nil), // 9: iop.RegisterResponse + (*NodeConfigPayload)(nil), // 10: iop.NodeConfigPayload + (*AdapterConfig)(nil), // 11: iop.AdapterConfig + (*NodeRuntimeConfig)(nil), // 12: iop.NodeRuntimeConfig + nil, // 13: iop.RunRequest.MetadataEntry + nil, // 14: iop.RunEvent.MetadataEntry + (*structpb.Struct)(nil), // 15: google.protobuf.Struct } var file_proto_iop_runtime_proto_depIdxs = []int32{ - 13, // 0: iop.RunRequest.policy:type_name -> google.protobuf.Struct - 13, // 1: iop.RunRequest.input:type_name -> google.protobuf.Struct - 11, // 2: iop.RunRequest.metadata:type_name -> iop.RunRequest.MetadataEntry - 2, // 3: iop.RunEvent.usage:type_name -> iop.Usage - 12, // 4: iop.RunEvent.metadata:type_name -> iop.RunEvent.MetadataEntry - 8, // 5: iop.RegisterResponse.config:type_name -> iop.NodeConfigPayload - 9, // 6: iop.NodeConfigPayload.adapters:type_name -> iop.AdapterConfig - 10, // 7: iop.NodeConfigPayload.runtime:type_name -> iop.NodeRuntimeConfig - 13, // 8: iop.AdapterConfig.settings:type_name -> google.protobuf.Struct - 9, // [9:9] is the sub-list for method output_type - 9, // [9:9] is the sub-list for method input_type - 9, // [9:9] is the sub-list for extension type_name - 9, // [9:9] is the sub-list for extension extendee - 0, // [0:9] is the sub-list for field type_name + 15, // 0: iop.RunRequest.policy:type_name -> google.protobuf.Struct + 15, // 1: iop.RunRequest.input:type_name -> google.protobuf.Struct + 13, // 2: iop.RunRequest.metadata:type_name -> iop.RunRequest.MetadataEntry + 0, // 3: iop.RunRequest.session_mode:type_name -> iop.RunSessionMode + 4, // 4: iop.RunEvent.usage:type_name -> iop.Usage + 14, // 5: iop.RunEvent.metadata:type_name -> iop.RunEvent.MetadataEntry + 1, // 6: iop.CancelRequest.action:type_name -> iop.CancelAction + 10, // 7: iop.RegisterResponse.config:type_name -> iop.NodeConfigPayload + 11, // 8: iop.NodeConfigPayload.adapters:type_name -> iop.AdapterConfig + 12, // 9: iop.NodeConfigPayload.runtime:type_name -> iop.NodeRuntimeConfig + 15, // 10: iop.AdapterConfig.settings:type_name -> google.protobuf.Struct + 11, // [11:11] is the sub-list for method output_type + 11, // [11:11] is the sub-list for method input_type + 11, // [11:11] is the sub-list for extension type_name + 11, // [11:11] is the sub-list for extension extendee + 0, // [0:11] is the sub-list for field type_name } func init() { file_proto_iop_runtime_proto_init() } @@ -825,13 +1026,14 @@ func file_proto_iop_runtime_proto_init() { File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_proto_iop_runtime_proto_rawDesc), len(file_proto_iop_runtime_proto_rawDesc)), - NumEnums: 0, + NumEnums: 2, NumMessages: 13, NumExtensions: 0, NumServices: 0, }, GoTypes: file_proto_iop_runtime_proto_goTypes, DependencyIndexes: file_proto_iop_runtime_proto_depIdxs, + EnumInfos: file_proto_iop_runtime_proto_enumTypes, MessageInfos: file_proto_iop_runtime_proto_msgTypes, }.Build() File_proto_iop_runtime_proto = out.File diff --git a/proto/iop/runtime.proto b/proto/iop/runtime.proto index 67a2aa0..f4b913b 100644 --- a/proto/iop/runtime.proto +++ b/proto/iop/runtime.proto @@ -6,6 +6,18 @@ import "google/protobuf/struct.proto"; option go_package = "iop/proto/gen/iop"; +enum RunSessionMode { + RUN_SESSION_MODE_UNSPECIFIED = 0; // default: create if missing + RUN_SESSION_MODE_CREATE_IF_MISSING = 1; + RUN_SESSION_MODE_REQUIRE_EXISTING = 2; +} + +enum CancelAction { + CANCEL_ACTION_UNSPECIFIED = 0; // default: cancel run only + CANCEL_ACTION_CANCEL_RUN = 1; + CANCEL_ACTION_TERMINATE_SESSION = 2; +} + // RunRequest initiates a model execution. message RunRequest { string run_id = 1; @@ -16,18 +28,23 @@ message RunRequest { google.protobuf.Struct input = 6; int32 timeout_sec = 7; map metadata = 8; + string session_id = 9; + RunSessionMode session_mode = 10; + bool background = 11; } // RunEvent is a streaming execution event. message RunEvent { string run_id = 1; - string type = 2; // start | delta | complete | error + string type = 2; // start | delta | complete | error | cancelled string delta = 3; string message = 4; string error = 5; Usage usage = 6; map metadata = 7; int64 timestamp = 8; // unix nano + string session_id = 9; + bool background = 10; } message Usage { @@ -42,7 +59,11 @@ message Heartbeat { // CancelRequest asks the node to cancel a running execution. message CancelRequest { - string run_id = 1; + string run_id = 1; + string adapter = 2; + string model = 3; + string session_id = 4; + CancelAction action = 5; } // Error is returned when a request fails at the transport layer.