From 51c809857726058334fe780766e2664fcb14dd53 Mon Sep 17 00:00:00 2001 From: toki Date: Sun, 14 Jun 2026 05:03:42 +0900 Subject: [PATCH] feat: operation event outbox - model, storage, migration, roadmap update - Add OperationEvent model with outbox pattern support - Extend storage layer with outbox query methods - Add migration for operation_event_outbox table - Update roadmap milestone progress - Add task documentation and planning files --- .../milestones/operation-event-outbox.md | 2 +- .../code_review_cloud_G05_0.log | 200 ++++++++++++++ .../code_review_cloud_G05_1.log | 194 ++++++++++++++ .../01_operation_store/complete.log | 44 +++ .../01_operation_store/plan_local_G06_0.log | 232 ++++++++++++++++ .../01_operation_store/plan_local_G06_1.log | 135 ++++++++++ .../CODE_REVIEW-cloud-G06.md | 153 +++++++++++ .../02+01_operation_pick/PLAN-local-G06.md | 233 ++++++++++++++++ services/core/internal/core/model.go | 50 +++- services/core/internal/core/model_test.go | 62 +++++ services/core/internal/storage/postgres.go | 250 +++++++++++++++++- .../internal/storage/postgres_repo_test.go | 105 ++++++++ .../core/internal/storage/postgres_test.go | 150 +++++++++++ services/core/internal/storage/storage.go | 30 ++- .../core/internal/storage/storage_test.go | 19 +- services/core/migrations/00001_initial.sql | 8 +- 16 files changed, 1847 insertions(+), 20 deletions(-) create mode 100644 agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_0.log create mode 100644 agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_1.log create mode 100644 agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/complete.log create mode 100644 agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_0.log create mode 100644 agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_1.log create mode 100644 agent-task/m-operation-event-outbox/02+01_operation_pick/CODE_REVIEW-cloud-G06.md create mode 100644 agent-task/m-operation-event-outbox/02+01_operation_pick/PLAN-local-G06.md create mode 100644 services/core/internal/core/model_test.go diff --git a/agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md b/agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md index 436c196..efbb9f5 100644 --- a/agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md +++ b/agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md @@ -36,7 +36,7 @@ operation lifecycle과 normalized event를 PostgreSQL 원장에 기록하고, pr Git operation, agent run, provider projection을 같은 operation 원장으로 관리한다. -- [ ] [op-model] operation type/state/idempotency schema를 확정한다. +- [x] [op-model] operation type/state/idempotency schema를 확정한다. - [ ] [op-store] operation create/start/succeed/fail/cancel store boundary를 만든다. 검증: invalid transition과 idempotency duplicate를 테스트한다. - [ ] [op-pick] worker가 pending operation을 안전하게 pick하는 기준을 만든다. 검증: 같은 operation이 중복 pick되지 않는다. diff --git a/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_0.log b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_0.log new file mode 100644 index 0000000..94a438e --- /dev/null +++ b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_0.log @@ -0,0 +1,200 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT - READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a user-only decision, user-owned external environment prerequisite, or scope conflict, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Evidence gaps that a follow-up agent can close by rerunning commands or collecting artifacts are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record the needed decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-14 +task=m-operation-event-outbox/01_operation_store, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- Task ids: + - `op-store`: operation create/start/succeed/fail/cancel store boundary를 만든다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G05.md` -> `code_review_cloud_G05_N.log`, `PLAN-local-G06.md` -> `plan_local_G06_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-operation-event-outbox/01_operation_store/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. +4. PASS이고 task group이 `m-operation-event-outbox`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] OperationStore lifecycle API | [x] | +| [API-2] Postgres operation store | [x] | + +## 구현 체크리스트 + +- [x] `OperationStore` 인터페이스를 create/get/list/start/succeed/fail/cancel 경계로 바꾸고 기존 fake/test compile break를 정리한다. +- [x] `PgStore.Operations()`가 실제 Postgres operation store를 반환하게 구현한다. +- [x] operation normalization, idempotency duplicate, invalid transition 테스트를 작성한다. 검증: invalid transition과 idempotency duplicate를 테스트한다. +- [x] `go test -count=1 ./internal/core ./internal/storage`와 `go test -count=1 ./...`를 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_{review_lane}_GNN_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_{build_lane}_GNN_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [ ] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [ ] PASS이면 active task 디렉터리 `agent-task/m-operation-event-outbox/01_operation_store/`를 `agent-task/archive/YYYY/MM/m-operation-event-outbox/01_operation_store/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-operation-event-outbox`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-operation-event-outbox/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [x] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-{build_lane}-GNN.md`와 `CODE_REVIEW-{review_lane}-GNN.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +- 계획 명령(`go test -count=1 ./internal/storage`, `./internal/core ./internal/storage`, `./...`)을 그대로 실행했다. 임의 대체 없음. 추가로 회귀 안전을 위해 `go vet ./internal/storage ./internal/core`를 실행했고 통과했다(계약 명령 대체가 아니라 보조 확인). +- Postgres integration 테스트(`TestPostgresOperationStoreLifecycleAndIdempotency`)는 `GITO_TEST_DATABASE_URL` 미설정으로 기존 테스트 계약대로 skip되었다. 이는 후속 에이전트가 DB 환경에서 재실행으로 채울 수 있는 증거 공백이며 사용자 리뷰 요청 사유가 아니다. + +## 주요 설계 결정 + +- `CreateOperation`의 idempotency conflict 처리: `idempotency_key`가 있으면 unique conflict에서 `ux_operations_idempotency` 부분 인덱스에 걸린 기존 row를 `getByIdempotencyKey`로 조회해 `Created=false`로 반환한다. `idempotency_key`가 없는 id PK collision은 정상 진행이 아니므로 `ErrOperationAlreadyExists`로 구분한다. +- 상태 전환은 single source of truth로 `core.OperationState.CanTransitionTo`를 사용한다. `transition` 헬퍼가 current row를 읽고 검증한 뒤 `UPDATE ... WHERE id=$1 AND state=$current`로 저장한다. read와 update 사이 상태가 바뀌면 `RETURNING`이 no rows가 되고 이를 `ErrInvalidOperation`(optimistic guard)로 변환한다. +- `normalizeOperation`은 빈 `State`를 `queued`로 보정하되, queued 이외 create state는 거부한다. create 경계가 항상 queued에서 시작하도록 강제한다. +- `ListOperations` ordering은 `created_at, id`로 고정해 `created_at` 동률 시에도 결정적이다. +- 마이그레이션 `00001_initial.sql`의 operations 테이블/제약(type/state CHECK, idempotency partial unique index)은 이미 정리되어 있어 구현 쿼리와 일치함을 확인만 했고 변경하지 않았다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- `CreateOperation` duplicate idempotency path가 기존 row를 반환하고 새 row를 만들지 않는지 확인한다. +- 상태 전환은 `queued -> running -> terminal`만 허용되는지 확인한다. +- `PgStore.Operations()`가 nil을 반환하지 않고 migration schema와 scanner가 일치하는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. + +### API-1 중간 검증 + +```text +$ cd services/core && go test -count=1 ./internal/storage +ok git.toki-labs.com/toki/gito/services/core/internal/storage 0.006s +``` + +### API-2 중간 검증 + +```text +$ cd services/core && go test -count=1 ./internal/storage +ok git.toki-labs.com/toki/gito/services/core/internal/storage 0.006s +``` + +### 최종 검증 + +```text +$ cd services/core && go test -count=1 ./internal/core ./internal/storage +ok git.toki-labs.com/toki/gito/services/core/internal/core 0.002s +ok git.toki-labs.com/toki/gito/services/core/internal/storage 0.004s + +$ cd services/core && go test -count=1 ./... +? git.toki-labs.com/toki/gito/services/core/cmd/server [no test files] +? git.toki-labs.com/toki/gito/services/core/cmd/shell [no test files] +? git.toki-labs.com/toki/gito/services/core/cmd/worker [no test files] +? git.toki-labs.com/toki/gito/services/core/internal/agentshell [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/config 0.002s +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.267s +ok git.toki-labs.com/toki/gito/services/core/internal/core 0.004s +? git.toki-labs.com/toki/gito/services/core/internal/events [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/gitengine 0.831s +ok git.toki-labs.com/toki/gito/services/core/internal/protosocket 0.005s +? git.toki-labs.com/toki/gito/services/core/internal/provider [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/provider/forgejo 0.004s +ok git.toki-labs.com/toki/gito/services/core/internal/storage 0.006s +? git.toki-labs.com/toki/gito/services/core/internal/worker [no test files] +``` + +> Postgres integration 케이스(`TestPostgresOperationStoreLifecycleAndIdempotency` 등)는 `GITO_TEST_DATABASE_URL` 미설정으로 기존 계약대로 skip되었다. + +--- + +> **[IMPLEMENTING AGENT - BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 섹션 소유권 + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, 개요, 리뷰 에이전트 지시 | Fixed at stub creation | 구현 에이전트는 종결 절차를 실행하지 않는다. | +| Roadmap Targets | Fixed at stub creation from plan | PASS 때 code-review가 `complete.log`에 복사한다. | +| 구현 항목별 완료 여부 | Implementing agent | 완료 항목만 체크한다. | +| 구현 체크리스트 | Implementing agent | 문구와 순서를 바꾸지 않는다. | +| 코드리뷰 전용 체크리스트 | Review agent only | 구현 에이전트는 수정하지 않는다. | +| 계획 대비 변경 사항, 주요 설계 결정 | Implementing agent | 실제 내용을 기록한다. | +| 사용자 리뷰 요청 | Implementing agent | 필요 없으면 `상태: 없음` 유지. | +| 리뷰어를 위한 체크포인트 | Fixed at stub creation | 리뷰 초점. | +| 검증 결과 | Implementing agent | 실제 stdout/stderr를 붙인다. | +| 코드리뷰 결과 | Review agent appends | Stub에는 포함하지 않는다. | + +## 코드리뷰 결과 + +### 종합 판정 + +FAIL + +### 차원별 평가 + +| 차원 | 평가 | 근거 | +|------|------|------| +| Correctness | Fail | idempotency key가 있는 create 요청에서 PK 충돌이 발생하면 idempotency index 충돌로 오판한다. | +| Completeness | Fail | 계획의 “id collision은 `ErrOperationAlreadyExists`” 계약이 idempotency key가 있는 요청까지 포괄하지 못한다. | +| Test coverage | Fail | idempotency key는 다르지만 ID가 같은 duplicate create 케이스가 테스트에 없다. | +| API contract | Fail | `CreateOperation` duplicate id 계약이 호출자에게 `ErrOperationAlreadyExists` 대신 `ErrOperationNotFound` 계열 오류를 반환할 수 있다. | +| Code quality | Pass | 구조와 helper 배치는 기존 storage 패턴과 일관된다. | +| Plan deviation | Fail | 구현 설명은 id PK collision을 duplicate로 구분한다고 했지만 실제 unique violation 분기에서는 constraint를 구분하지 않는다. | +| Verification trust | Pass | 리뷰어 재실행 기준 `go test -count=1 ./internal/core ./internal/storage`, `go test -count=1 ./...`, `go vet ./internal/storage ./internal/core` 모두 통과했다. | + +### 발견된 문제 + +- Required: [services/core/internal/storage/postgres.go](/config/workspace/gito/services/core/internal/storage/postgres.go:527) `CreateOperation`이 모든 unique violation을 같은 방식으로 처리하고, `op.IdempotencyKey != ""`이면 무조건 `getByIdempotencyKey`를 호출한다. 이미 `id=op-001`인 row가 있고 새 요청이 `id=op-001`, `idempotency_key=other-key`처럼 다른 idempotency key를 보내면 실제 충돌은 operations PK인데도 새 key로 조회하다가 `ErrOperationNotFound`를 반환한다. `pgconn.PgError.ConstraintName`으로 `ux_operations_idempotency`와 `operations_pkey`를 구분해서 idempotency index 충돌만 기존 row를 반환하고, PK 충돌은 idempotency key 유무와 무관하게 `ErrOperationAlreadyExists`로 반환하도록 고쳐야 한다. [services/core/internal/storage/postgres_test.go](/config/workspace/gito/services/core/internal/storage/postgres_test.go:492)의 duplicate 테스트도 `id`는 같고 `idempotency_key`만 다른 케이스를 추가해야 한다. + +### 다음 단계 + +- FAIL 후속 plan/review를 작성한다. user-review gate는 트리거하지 않는다. diff --git a/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_1.log b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_1.log new file mode 100644 index 0000000..2ad7711 --- /dev/null +++ b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_1.log @@ -0,0 +1,194 @@ + + +# Code Review Reference - REVIEW_API + +> **[IMPLEMENTING AGENT - READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a user-only decision, user-owned external environment prerequisite, or scope conflict, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Evidence gaps that a follow-up agent can close by rerunning commands or collecting artifacts are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record the needed decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-14 +task=m-operation-event-outbox/01_operation_store, plan=1, tag=REVIEW_API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- Task ids: + - `op-store`: operation create/start/succeed/fail/cancel store boundary를 만든다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G05.md` -> `code_review_cloud_G05_N.log`, `PLAN-local-G06.md` -> `plan_local_G06_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-operation-event-outbox/01_operation_store/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. `USER_REVIEW.md`가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log` 작성 후 archive 이동한다. +4. PASS이고 task group이 `m-operation-event-outbox`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_API-1] Unique violation constraint routing | [x] | + +## 구현 체크리스트 + +- [x] `CreateOperation`이 Postgres unique violation의 constraint를 구분해 `ux_operations_idempotency`일 때만 기존 idempotency row를 반환하고, `operations_pkey` 등 id collision은 idempotency key 유무와 무관하게 `ErrOperationAlreadyExists`로 반환하게 한다. +- [x] id는 같고 idempotency key만 다른 duplicate create 테스트를 추가해 `ErrOperationAlreadyExists`를 검증한다. +- [x] `go test -count=1 ./internal/storage`와 `go test -count=1 ./internal/core ./internal/storage`와 `go test -count=1 ./...`를 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_{review_lane}_GNN_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_{build_lane}_GNN_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [x] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [x] PASS이면 active task 디렉터리 `agent-task/m-operation-event-outbox/01_operation_store/`를 `agent-task/archive/YYYY/MM/m-operation-event-outbox/01_operation_store/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [x] PASS이고 task group이 `m-operation-event-outbox`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [x] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-operation-event-outbox/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-{build_lane}-GNN.md`와 `CODE_REVIEW-{review_lane}-GNN.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +- 계획의 고정 검증 명령(`go test -count=1 ./internal/storage`, `./internal/core ./internal/storage`, `./...`)을 그대로 실행했다. 임의 대체 없음. +- 계획은 `uniqueViolationConstraint` helper를 새로 추가하거나 기존 `isUniqueViolation`을 확장하는 두 안을 제시했다. 둘을 결합해 `uniqueViolationConstraint(err) (string, bool)`를 단일 source로 두고 `isUniqueViolation`이 이를 호출하도록 했다. 기존 `isUniqueViolation` 호출부(repo/lease 등)의 동작은 그대로 유지된다. + +## 주요 설계 결정 + +- constraint 분기 기준은 `pgconn.PgError.ConstraintName`이다. `ux_operations_idempotency`(migration의 partial unique index) 충돌이고 요청 idempotency key가 비어 있지 않을 때만 기존 row를 idempotent retry로 반환한다. `operations_pkey` 등 그 외 unique violation은 idempotency key 유무와 무관하게 `ErrOperationAlreadyExists`로 반환한다. 이로써 "같은 id + 다른 idempotency key" 요청이 잘못된 key로 기존 row를 조회해 `ErrOperationNotFound` 계열 오류를 반환하던 결함을 제거했다. +- 인덱스 이름을 `opIdempotencyConstraint` 상수로 두어 migration의 인덱스 이름과 코드의 분기 기준을 한 곳에서 맞춘다. +- DB 없는 환경에서도 핵심 분기를 검증하도록 `uniqueViolationConstraint`/`isUniqueViolation`을 `pgconn.PgError{Code:"23505", ConstraintName:...}` 직접 주입으로 unit-test했다(`TestUniqueViolationConstraintRouting`). idempotency index, pkey, 23503 FK, non-pg error를 모두 커버한다. + +## 검증 증거 비고 + +- Integration 테스트(`TestPostgresOperationStoreLifecycleAndIdempotency`, "same id different idempotency key" 케이스 포함)는 `GITO_TEST_DATABASE_URL` 미설정으로 기존 계약대로 skip되었다. 이는 후속 에이전트가 DB 환경에서 재실행으로 채울 수 있는 증거 공백이며, 계획이 요구한 DB 없는 constraint routing 단위 테스트는 실제로 실행되어 통과했다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- `CreateOperation`이 `pgconn.PgError.ConstraintName` 또는 동등한 근거로 idempotency index 충돌과 PK 충돌을 구분하는지 확인한다. +- 같은 operation id와 다른 idempotency key로 create할 때 `ErrOperationAlreadyExists`가 반환되는지 확인한다. +- DB 없는 local 환경에서도 constraint routing 테스트가 실제로 실행되는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. +- mobile/UI hang, timeout, 또는 2분 무진행은 blind retry를 중단하고 focused rerun 명령과 screenshot/window/UI-tree evidence path를 남기며, 불가능하면 정확한 사유를 남긴다. + +### REVIEW_API-1 중간 검증 + +```text +$ cd services/core && go test -count=1 ./internal/storage +ok git.toki-labs.com/toki/gito/services/core/internal/storage 0.004s +``` + +### 최종 검증 + +```text +$ cd services/core && go test -count=1 ./internal/core ./internal/storage +ok git.toki-labs.com/toki/gito/services/core/internal/core 0.002s +ok git.toki-labs.com/toki/gito/services/core/internal/storage 0.004s + +$ cd services/core && go test -count=1 ./... +? git.toki-labs.com/toki/gito/services/core/cmd/server [no test files] +? git.toki-labs.com/toki/gito/services/core/cmd/shell [no test files] +? git.toki-labs.com/toki/gito/services/core/cmd/worker [no test files] +? git.toki-labs.com/toki/gito/services/core/internal/agentshell [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/config 0.002s +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.259s +ok git.toki-labs.com/toki/gito/services/core/internal/core 0.002s +? git.toki-labs.com/toki/gito/services/core/internal/events [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/gitengine 0.938s +ok git.toki-labs.com/toki/gito/services/core/internal/protosocket 0.004s +? git.toki-labs.com/toki/gito/services/core/internal/provider [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/provider/forgejo 0.002s +ok git.toki-labs.com/toki/gito/services/core/internal/storage 0.004s +? git.toki-labs.com/toki/gito/services/core/internal/worker [no test files] +``` + +> Postgres integration 케이스는 `GITO_TEST_DATABASE_URL` 미설정으로 기존 계약대로 skip되었다. DB 없이 실행되는 `TestUniqueViolationConstraintRouting`은 통과했다. + +--- + +> **[IMPLEMENTING AGENT - BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 섹션 소유권 + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, 개요, 리뷰 에이전트 지시 | Fixed at stub creation | 구현 에이전트는 종결 절차를 실행하지 않는다. | +| Roadmap Targets | Fixed at stub creation from plan | PASS 때 code-review가 `complete.log`에 복사한다. | +| 구현 항목별 완료 여부 | Implementing agent | 완료 항목만 체크한다. | +| 구현 체크리스트 | Implementing agent | 문구와 순서를 바꾸지 않는다. | +| 코드리뷰 전용 체크리스트 | Review agent only | 구현 에이전트는 수정하지 않는다. | +| 계획 대비 변경 사항, 주요 설계 결정 | Implementing agent | 실제 내용을 기록한다. | +| 사용자 리뷰 요청 | Implementing agent | 필요 없으면 `상태: 없음` 유지. | +| 리뷰어를 위한 체크포인트 | Fixed at stub creation | 리뷰 초점. | +| 검증 결과 | Implementing agent | 실제 stdout/stderr를 붙인다. | +| 코드리뷰 결과 | Review agent appends | Stub에는 포함하지 않는다. | + +## 코드리뷰 결과 + +### 종합 판정 + +PASS + +### 차원별 평가 + +| 차원 | 평가 | 근거 | +|------|------|------| +| Correctness | Pass | `CreateOperation`이 unique violation constraint를 확인해 idempotency index 충돌만 retry로 처리하고, PK 충돌은 idempotency key 유무와 무관하게 `ErrOperationAlreadyExists`로 반환한다. | +| Completeness | Pass | 후속 계획의 constraint routing, same-id/different-idempotency-key 테스트, 검증 기록 항목이 모두 완료됐다. | +| Test coverage | Pass | DB 없는 `TestUniqueViolationConstraintRouting`이 constraint 분기를 검증하고, Postgres integration 테스트에도 same-id/different-key duplicate 케이스가 추가됐다. | +| API contract | Pass | 기존 `isUniqueViolation` 호출부 동작은 유지하면서 operation create duplicate 계약을 보강했다. | +| Code quality | Pass | 변경 범위가 작고 기존 storage helper 패턴과 일관된다. | +| Plan deviation | Pass | 계획에서 허용한 helper 추가/확장 방향 안에서 구현됐다. | +| Verification trust | Pass | 리뷰어 재실행 기준 `go test -count=1 ./internal/storage`, `go test -count=1 ./internal/core ./internal/storage`, `go test -count=1 ./...`, `go vet ./internal/storage ./internal/core` 모두 통과했다. | + +### 발견된 문제 + +없음 + +### 다음 단계 + +- PASS: `complete.log` 작성 후 task 디렉터리를 archive로 이동한다. diff --git a/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/complete.log b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/complete.log new file mode 100644 index 0000000..f4ce0a4 --- /dev/null +++ b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/complete.log @@ -0,0 +1,44 @@ +# Complete - m-operation-event-outbox/01_operation_store + +## 완료 일시 + +2026-06-14 + +## 요약 + +Operation store boundary 구현 및 duplicate conflict follow-up을 2회 리뷰 루프로 완료했다. Final verdict: PASS. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_local_G06_0.log` | `code_review_cloud_G05_0.log` | FAIL | `CreateOperation` unique violation 분기가 PK 충돌과 idempotency index 충돌을 구분하지 않아 follow-up 필요. | +| `plan_local_G06_1.log` | `code_review_cloud_G05_1.log` | PASS | constraint-aware duplicate routing과 same-id/different-idempotency-key 테스트 보강 완료. | + +## 구현/정리 내용 + +- `OperationStore`를 create/get/list/start/succeed/fail/cancel lifecycle boundary로 확장하고 `PgStore.Operations()`가 Postgres-backed operation store를 반환하도록 구현했다. +- operation create normalization, idempotency duplicate, invalid transition, list/get behavior를 storage 테스트로 보강했다. +- `CreateOperation` unique violation 처리에서 `ux_operations_idempotency`와 PK 충돌을 구분해 duplicate create 계약을 정리했다. + +## 최종 검증 + +- `cd services/core && go test -count=1 ./internal/storage` - PASS; `ok git.toki-labs.com/toki/gito/services/core/internal/storage`. +- `cd services/core && go test -count=1 ./internal/core ./internal/storage` - PASS; core/storage packages passed. +- `cd services/core && go test -count=1 ./...` - PASS; all packages passed or reported `[no test files]`. +- `cd services/core && go vet ./internal/storage ./internal/core` - PASS; no output. + +## Roadmap Completion + +- Milestone: `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- Completed task ids: + - `op-store`: PASS; evidence=`plan_local_G06_0.log`, `code_review_cloud_G05_0.log`, `plan_local_G06_1.log`, `code_review_cloud_G05_1.log`; verification=`cd services/core && go test -count=1 ./internal/storage`, `cd services/core && go test -count=1 ./internal/core ./internal/storage`, `cd services/core && go test -count=1 ./...` +- Not completed task ids: 없음 + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_0.log b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_0.log new file mode 100644 index 0000000..1ecebf7 --- /dev/null +++ b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_0.log @@ -0,0 +1,232 @@ + + +# Operation Store Boundary + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채워야 구현이 끝난다. 검증 명령을 실행하고 출력 원문을 붙인 뒤 active 파일을 그대로 두고 리뷰 준비를 보고한다. 종결, log rename, `complete.log`, archive 이동은 code-review-skill 전용이다. 사용자 결정, 사용자 소유 외부 환경, scope 충돌 없이는 진행할 수 없으면 review stub의 `사용자 리뷰 요청`에 근거를 기록하고 중단한다. 구현 중 사용자에게 직접 묻거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 재실행으로 채울 수 있는 검증 증거 공백은 사용자 리뷰 요청이 아니다. + +## 배경 + +현재 operation type/state/idempotency 모델과 SQL 제약은 정리됐지만, `storage.OperationStore`는 `SaveOperation` 수준이고 `PgStore.Operations()`는 `nil`을 반환한다. `op-store` 마일스톤 task는 create/start/succeed/fail/cancel 전환과 idempotency duplicate 처리를 Postgres 원장 경계로 고정해야 한다. + +## 사용자 리뷰 요청 흐름 + +구현 중 차단은 active `CODE_REVIEW-*-G??.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 이 섹션은 `agent-ops/skills/common/_templates/implementation-user-review-request-section.md` 형식을 따른다. 직접 사용자 프롬프트는 금지되며, code-review가 요청 타당성 검증과 실제 `USER_REVIEW.md` 작성을 맡는다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- Task ids: + - `op-store`: operation create/start/succeed/fail/cancel store boundary를 만든다. +- Completion mode: check-on-pass + +## 분석 결과 + +### 읽은 파일 + +- `agent-roadmap/current.md` +- `agent-roadmap/phase/control-plane-foundation/PHASE.md` +- `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- `agent-ops/rules/project/domain/core/rules.md` +- `agent-test/local/rules.md` +- `agent-test/local/core-smoke.md` +- `services/core/go.mod` +- `services/core/internal/core/model.go` +- `services/core/internal/core/model_test.go` +- `services/core/internal/storage/storage.go` +- `services/core/internal/storage/postgres.go` +- `services/core/internal/storage/storage_test.go` +- `services/core/internal/storage/postgres_test.go` +- `services/core/internal/storage/postgres_repo_test.go` +- `services/core/migrations/00001_initial.sql` + +### 테스트 환경 규칙 + +- test_env: `local`. +- `agent-test/local/rules.md` present/read. Matching profile: `agent-test/local/core-smoke.md`. +- Applied command contract: core changes require `cd services/core && go test ./...`; optional `go vet ./...`/`go build ./...` may be added, but final verification here uses fresh `go test -count=1 ./...`. +- No `<확인 필요>` command values were found. Postgres integration tests depend on `GITO_TEST_DATABASE_URL`; when unset they skip by existing test contract. + +### 테스트 커버리지 공백 + +- Operation create/idempotency duplicate: missing today. Add storage unit validation and Postgres integration coverage. +- Operation invalid transitions: core state helpers now exist, but store transition methods have no tests. Add focused transition tests. +- Operation list/get scan ordering: missing today. Add integration assertions. + +### 심볼 참조 + +- Planned interface change removes/replaces `SaveOperation`; call sites found only in `services/core/internal/storage/storage_test.go` fake implementation. No production call sites exist. + +### 분할 판단 + +- Split policy evaluated before file selection. +- Shared task group: `agent-task/m-operation-event-outbox/`. +- `01_operation_store`: independent foundation. Implements operation store boundary. +- `02+01_operation_pick`: depends on `01_operation_store` because picking needs durable operation state transitions. +- This file has no predecessor. + +### 범위 결정 근거 + +- Exclude worker pick and runner behavior; handled by `02+01_operation_pick`. +- Exclude durable outbox event append/list/publish; belongs to outbox epic. +- Exclude proto-socket contracts/UI/provider projection; out of this milestone task. + +### 빌드 등급 + +- Build: `local-G06`. Review: `cloud-G05`. +- Rationale: bounded Go storage implementation, but storage/migration/idempotency state transitions need careful review and deterministic tests. + +## 구현 체크리스트 + +- [ ] `OperationStore` 인터페이스를 create/get/list/start/succeed/fail/cancel 경계로 바꾸고 기존 fake/test compile break를 정리한다. +- [ ] `PgStore.Operations()`가 실제 Postgres operation store를 반환하게 구현한다. +- [ ] operation normalization, idempotency duplicate, invalid transition 테스트를 작성한다. 검증: invalid transition과 idempotency duplicate를 테스트한다. +- [ ] `go test -count=1 ./internal/core ./internal/storage`와 `go test -count=1 ./...`를 실행한다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [API-1] OperationStore lifecycle API + +#### 문제 + +- [services/core/internal/storage/storage.go](/config/workspace/gito/services/core/internal/storage/storage.go:53)의 `OperationStore`는 `SaveOperation`만 있어 create/start/succeed/fail/cancel 상태 전환과 idempotency duplicate 결과를 표현하지 못한다. + +Before: + +```go +53 type OperationStore interface { +54 SaveOperation(ctx context.Context, op core.Operation) error +55 GetOperation(ctx context.Context, id string) (core.Operation, error) +56 ListOperations(ctx context.Context, repoID string) ([]core.Operation, error) +57 } +``` + +#### 해결 방법 + +- `ErrInvalidOperation`, `ErrOperationAlreadyExists`, `ErrOperationNotFound`를 추가한다. +- `OperationCreateResult{Operation core.Operation, Created bool}`를 추가한다. +- `CreateOperation`, `GetOperation`, `ListOperations`, `StartOperation`, `SucceedOperation`, `FailOperation`, `CancelOperation`으로 interface를 확정한다. +- `Start/Succeed/Fail/Cancel`은 `core.OperationState.CanTransitionTo`를 기준으로 invalid transition을 거부한다. + +After: + +```go +type OperationCreateResult struct { + Operation core.Operation + Created bool +} + +type OperationStore interface { + CreateOperation(ctx context.Context, op core.Operation) (OperationCreateResult, error) + GetOperation(ctx context.Context, id string) (core.Operation, error) + ListOperations(ctx context.Context, repoID string) ([]core.Operation, error) + StartOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) + SucceedOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) + FailOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) + CancelOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) +} +``` + +#### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/storage/storage.go`: errors/result/interface 갱신. +- [ ] `services/core/internal/storage/storage_test.go`: fakeOps와 boundary contract compile check 갱신. + +#### 테스트 작성 + +- `services/core/internal/storage/storage_test.go`에서 interface compile check를 유지한다. + +#### 중간 검증 + +```bash +cd services/core && go test -count=1 ./internal/storage +``` + +Expected: storage package passes or Postgres integration cases skip only when `GITO_TEST_DATABASE_URL` is unset. + +### [API-2] Postgres operation store + +#### 문제 + +- [services/core/internal/storage/postgres.go](/config/workspace/gito/services/core/internal/storage/postgres.go:19)의 `PgStore`에는 operation sub-store field가 없고, [services/core/internal/storage/postgres.go](/config/workspace/gito/services/core/internal/storage/postgres.go:61)는 `nil`을 반환한다. + +Before: + +```go +19 type PgStore struct { +20 pool *pgxpool.Pool +21 repos *pgRepoStore +22 leases *pgWorkspaceLeaseStore +23 watches *pgBranchWatchStore +24 cursors *pgRevisionCursorStore +25 deliveries *pgProviderDeliveryStore +} +... +61 func (s *PgStore) Operations() OperationStore { return nil } +``` + +#### 해결 방법 + +- `ops *pgOperationStore` field를 추가하고 `NewPgStore`에서 초기화한다. +- `pgOperationStore`를 `pgWorkspaceLeaseStore`와 같은 파일에 추가한다. +- `CreateOperation`은 normalize 후 insert한다. `idempotency_key`가 있으면 unique conflict에서 기존 row를 select하고 `Created=false`를 반환한다. id collision은 `ErrOperationAlreadyExists`로 반환한다. +- `Start/Succeed/Fail/Cancel`은 current row를 읽고 `CanTransitionTo` 검증 후 `UPDATE ... WHERE id=$1 AND state=$old`로 저장한다. +- `ListOperations`는 `repo_id` 필터와 `created_at, id` ordering을 사용한다. + +After: + +```go +type PgStore struct { + pool *pgxpool.Pool + repos *pgRepoStore + leases *pgWorkspaceLeaseStore + ops *pgOperationStore + watches *pgBranchWatchStore + cursors *pgRevisionCursorStore + deliveries *pgProviderDeliveryStore +} + +func (s *PgStore) Operations() OperationStore { return s.ops } +``` + +#### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/storage/postgres.go`: `pgOperationStore`, normalization, scanner, lifecycle methods 구현. +- [ ] `services/core/internal/storage/postgres_repo_test.go`: DB 없이 가능한 invalid operation normalization 테스트 추가. +- [ ] `services/core/internal/storage/postgres_test.go`: DB integration으로 create/list/get/idempotency duplicate/transition 테스트 추가. +- [ ] `services/core/migrations/00001_initial.sql`: 이미 정리된 operation constraints와 구현 쿼리가 맞는지 확인만 한다. + +#### 테스트 작성 + +- `TestPgOperationStoreValidatesBeforeDatabase`: required fields, invalid type/state, non-queued create state를 검증한다. +- `TestPostgresOperationStoreLifecycleAndIdempotency`: create duplicate idempotency returns existing with `Created=false`; queued->running->succeeded succeeds; succeeded->failed fails with `ErrInvalidOperation`; list ordering is stable. + +#### 중간 검증 + +```bash +cd services/core && go test -count=1 ./internal/storage +``` + +Expected: storage package passes; DB-specific tests may skip only when `GITO_TEST_DATABASE_URL` is unset. + +## 수정 파일 요약 + +| 파일 | 항목 | +| --- | --- | +| `services/core/internal/storage/storage.go` | API-1 | +| `services/core/internal/storage/storage_test.go` | API-1 | +| `services/core/internal/storage/postgres.go` | API-2 | +| `services/core/internal/storage/postgres_repo_test.go` | API-2 | +| `services/core/internal/storage/postgres_test.go` | API-2 | +| `services/core/migrations/00001_initial.sql` | API-2 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./internal/core ./internal/storage +cd services/core && go test -count=1 ./... +``` + +Expected: all packages pass; Postgres integration tests skip only if `GITO_TEST_DATABASE_URL` is unset. + +모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_1.log b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_1.log new file mode 100644 index 0000000..6470e90 --- /dev/null +++ b/agent-task/archive/2026/06/m-operation-event-outbox/01_operation_store/plan_local_G06_1.log @@ -0,0 +1,135 @@ + + +# Operation Store Duplicate Conflict Fix + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채워야 구현이 끝난다. 검증 명령을 실행하고 출력 원문을 붙인 뒤 active 파일을 그대로 두고 리뷰 준비를 보고한다. 종결, log rename, `complete.log`, archive 이동은 code-review-skill 전용이다. 사용자 결정, 사용자 소유 외부 환경, scope 충돌 없이는 진행할 수 없으면 review stub의 `사용자 리뷰 요청`에 근거를 기록하고 중단한다. 구현 중 사용자에게 직접 묻거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 재실행으로 채울 수 있는 검증 증거 공백은 사용자 리뷰 요청이 아니다. + +## 배경 + +이 후속 계획은 `code_review_cloud_G05_0.log`의 Required 이슈만 해결한다. `CreateOperation`은 idempotency duplicate와 operation id collision을 구분해야 한다. 현재 구현은 unique violation의 constraint를 보지 않고 `op.IdempotencyKey != ""`이면 idempotency duplicate로 간주해, 같은 `id`와 다른 `idempotency_key` 요청에서 잘못된 오류를 반환할 수 있다. + +## 사용자 리뷰 요청 흐름 + +구현 중 차단은 active `CODE_REVIEW-*-G??.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 이 섹션은 `agent-ops/skills/common/_templates/implementation-user-review-request-section.md` 형식을 따른다. 직접 사용자 프롬프트는 금지되며, code-review가 요청 타당성 검증과 실제 `USER_REVIEW.md` 작성을 맡는다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- Task ids: + - `op-store`: operation create/start/succeed/fail/cancel store boundary를 만든다. +- Completion mode: check-on-pass + +## 분석 결과 + +### 선행 리뷰 + +- `agent-task/m-operation-event-outbox/01_operation_store/code_review_cloud_G05_0.log` +- 판정: FAIL +- Required: `CreateOperation` unique violation 분기가 PK 충돌과 idempotency index 충돌을 구분하지 않는다. + +### 수정 대상 + +- `services/core/internal/storage/postgres.go` +- `services/core/internal/storage/postgres_test.go` +- 필요 시 `services/core/internal/storage/postgres_repo_test.go` + +### 범위 결정 근거 + +- operation store duplicate conflict 처리만 수정한다. +- worker pick, operation event outbox, provider adapter, client, roadmap 문서 갱신은 제외한다. +- DB가 없는 local 환경에서도 핵심 분기 테스트가 가능하도록 constraint classification은 작은 helper로 분리하거나 동등한 방식으로 unit-testable하게 만든다. + +### 빌드 등급 + +- Build: `local-G06`. Review: `cloud-G05`. +- Rationale: storage correctness follow-up이지만 이슈가 한 지점으로 명확하고 deterministic test로 검증 가능하다. + +## 구현 체크리스트 + +- [ ] `CreateOperation`이 Postgres unique violation의 constraint를 구분해 `ux_operations_idempotency`일 때만 기존 idempotency row를 반환하고, `operations_pkey` 등 id collision은 idempotency key 유무와 무관하게 `ErrOperationAlreadyExists`로 반환하게 한다. +- [ ] id는 같고 idempotency key만 다른 duplicate create 테스트를 추가해 `ErrOperationAlreadyExists`를 검증한다. +- [ ] `go test -count=1 ./internal/storage`와 `go test -count=1 ./internal/core ./internal/storage`와 `go test -count=1 ./...`를 실행한다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [REVIEW_API-1] Unique violation constraint routing + +#### 문제 + +[services/core/internal/storage/postgres.go](/config/workspace/gito/services/core/internal/storage/postgres.go:527)의 unique violation 분기가 constraint 이름을 구분하지 않는다. + +Before: + +```go +if isUniqueViolation(err) { + if op.IdempotencyKey != "" { + existing, getErr := s.getByIdempotencyKey(ctx, op.IdempotencyKey) + ... + return OperationCreateResult{Operation: existing, Created: false}, nil + } + return OperationCreateResult{}, fmt.Errorf("%w: %s", ErrOperationAlreadyExists, op.ID) +} +``` + +`id=op-001`, `idempotency_key=key-a` row가 있는 상태에서 `id=op-001`, `idempotency_key=key-b`를 생성하면 실제 충돌은 `operations_pkey`다. 하지만 현재 코드는 `key-b`로 기존 row를 조회해 `ErrOperationNotFound` 계열 오류를 반환할 수 있다. + +#### 해결 방법 + +- `pgconn.PgError.ConstraintName`을 확인하는 helper를 추가하거나 기존 `isUniqueViolation`을 확장한다. +- `ux_operations_idempotency` 충돌이고 요청 idempotency key가 비어 있지 않을 때만 `getByIdempotencyKey`를 호출한다. +- `operations_pkey` 또는 그 외 create duplicate로 해석해야 하는 unique violation은 `ErrOperationAlreadyExists`로 반환한다. +- helper를 만들 경우 DB 없이 `pgconn.PgError{Code: "23505", ConstraintName: ...}`로 분기 테스트를 작성한다. + +After 예시: + +```go +constraint, unique := uniqueViolationConstraint(err) +if unique { + if constraint == "ux_operations_idempotency" && op.IdempotencyKey != "" { + existing, getErr := s.getByIdempotencyKey(ctx, op.IdempotencyKey) + ... + return OperationCreateResult{Operation: existing, Created: false}, nil + } + return OperationCreateResult{}, fmt.Errorf("%w: %s", ErrOperationAlreadyExists, op.ID) +} +``` + +#### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/storage/postgres.go`: constraint-aware unique violation 처리. +- [ ] `services/core/internal/storage/postgres_test.go`: 실제 Postgres integration duplicate id/different idempotency key 케이스 추가. +- [ ] `services/core/internal/storage/postgres_repo_test.go`: DB 없이 constraint routing helper 테스트 추가 또는 동등한 unit coverage 추가. + +#### 테스트 작성 + +- Integration: 기존 `TestPostgresOperationStoreLifecycleAndIdempotency`에 `ID`는 기존 row와 같고 `IdempotencyKey`만 다른 operation create를 추가해 `errors.Is(err, storage.ErrOperationAlreadyExists)`를 검증한다. +- Unit: helper를 추가했다면 `operations_pkey`는 duplicate id, `ux_operations_idempotency`는 idempotency duplicate로 분기되는지 확인한다. + +#### 중간 검증 + +```bash +cd services/core && go test -count=1 ./internal/storage +``` + +Expected: storage package passes; `GITO_TEST_DATABASE_URL`이 없으면 Postgres integration은 기존 계약대로 skip될 수 있으나, DB 없는 helper/unit coverage는 실행되어야 한다. + +## 수정 파일 요약 + +| 파일 | 항목 | +| --- | --- | +| `services/core/internal/storage/postgres.go` | REVIEW_API-1 | +| `services/core/internal/storage/postgres_test.go` | REVIEW_API-1 | +| `services/core/internal/storage/postgres_repo_test.go` | REVIEW_API-1 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./internal/storage +cd services/core && go test -count=1 ./internal/core ./internal/storage +cd services/core && go test -count=1 ./... +``` + +Expected: all packages pass; DB-specific tests may skip only if `GITO_TEST_DATABASE_URL` is unset, and DB 없이 실행되는 duplicate constraint 분기 테스트는 반드시 통과해야 한다. + +모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/m-operation-event-outbox/02+01_operation_pick/CODE_REVIEW-cloud-G06.md b/agent-task/m-operation-event-outbox/02+01_operation_pick/CODE_REVIEW-cloud-G06.md new file mode 100644 index 0000000..aff7f40 --- /dev/null +++ b/agent-task/m-operation-event-outbox/02+01_operation_pick/CODE_REVIEW-cloud-G06.md @@ -0,0 +1,153 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT - READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a user-only decision, user-owned external environment prerequisite, or scope conflict, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Evidence gaps that a follow-up agent can close by rerunning commands or collecting artifacts are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record the needed decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-06-14 +task=m-operation-event-outbox/02+01_operation_pick, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- Task ids: + - `op-pick`: worker가 pending operation을 안전하게 pick하는 기준을 만든다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G06.md` -> `code_review_cloud_G06_N.log`, `PLAN-local-G06.md` -> `plan_local_G06_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-operation-event-outbox/02+01_operation_pick/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. +4. PASS이고 task group이 `m-operation-event-outbox`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] Atomic pending operation pick | [ ] | +| [API-2] Worker run-one pick boundary | [ ] | + +## 구현 체크리스트 + +- [ ] `OperationStore`에 pending pick API를 추가하고 `pgOperationStore`에서 `FOR UPDATE SKIP LOCKED` 기반으로 queued operation을 running으로 전환한다. +- [ ] worker runner에 one-iteration pick boundary를 추가하고 picker가 없거나 worker disabled일 때의 동작을 테스트한다. +- [ ] concurrent pick 테스트를 작성한다. 검증: 같은 operation이 중복 pick되지 않는다. +- [ ] `go test -count=1 ./internal/storage ./internal/worker`와 `go test -count=1 ./...`를 실행한다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_{review_lane}_GNN_N.log`로 아카이브한다. +- [ ] active `PLAN-*-G??.md`를 `plan_{build_lane}_GNN_M.log`로 아카이브한다. +- [ ] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [ ] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [ ] PASS이면 active task 디렉터리 `agent-task/m-operation-event-outbox/02+01_operation_pick/`를 `agent-task/archive/YYYY/MM/m-operation-event-outbox/02+01_operation_pick/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-operation-event-outbox`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-operation-event-outbox/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-{build_lane}-GNN.md`와 `CODE_REVIEW-{review_lane}-GNN.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ + +## 주요 설계 결정 + +_구현 에이전트가 주요 설계 결정 사항을 기록한다._ + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- `02+01_operation_pick` 구현 전에 `01_operation_store/complete.log`가 존재했는지 확인한다. +- Pick SQL이 `FOR UPDATE SKIP LOCKED`를 사용하고 queued row를 running으로 atomically 전환하는지 확인한다. +- Worker runner가 disabled/no picker/no work/error/picked path를 모두 테스트하는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. + +### API-1 중간 검증 + +```text +$ cd services/core && go test -count=1 ./internal/storage +(output) +``` + +### API-2 중간 검증 + +```text +$ cd services/core && go test -count=1 ./internal/worker +(output) +``` + +### 최종 검증 + +```text +$ cd services/core && go test -count=1 ./internal/storage ./internal/worker +(output) + +$ cd services/core && go test -count=1 ./... +(output) +``` + +--- + +> **[IMPLEMENTING AGENT - BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 섹션 소유권 + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, 개요, 리뷰 에이전트 지시 | Fixed at stub creation | 구현 에이전트는 종결 절차를 실행하지 않는다. | +| Roadmap Targets | Fixed at stub creation from plan | PASS 때 code-review가 `complete.log`에 복사한다. | +| 구현 항목별 완료 여부 | Implementing agent | 완료 항목만 체크한다. | +| 구현 체크리스트 | Implementing agent | 문구와 순서를 바꾸지 않는다. | +| 코드리뷰 전용 체크리스트 | Review agent only | 구현 에이전트는 수정하지 않는다. | +| 계획 대비 변경 사항, 주요 설계 결정 | Implementing agent | 실제 내용을 기록한다. | +| 사용자 리뷰 요청 | Implementing agent | 필요 없으면 `상태: 없음` 유지. | +| 리뷰어를 위한 체크포인트 | Fixed at stub creation | 리뷰 초점. | +| 검증 결과 | Implementing agent | 실제 stdout/stderr를 붙인다. | +| 코드리뷰 결과 | Review agent appends | Stub에는 포함하지 않는다. | diff --git a/agent-task/m-operation-event-outbox/02+01_operation_pick/PLAN-local-G06.md b/agent-task/m-operation-event-outbox/02+01_operation_pick/PLAN-local-G06.md new file mode 100644 index 0000000..d2e63e0 --- /dev/null +++ b/agent-task/m-operation-event-outbox/02+01_operation_pick/PLAN-local-G06.md @@ -0,0 +1,233 @@ + + +# Pending Operation Pick Boundary + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채워야 구현이 끝난다. 검증 명령을 실행하고 출력 원문을 붙인 뒤 active 파일을 그대로 두고 리뷰 준비를 보고한다. 종결, log rename, `complete.log`, archive 이동은 code-review-skill 전용이다. 사용자 결정, 사용자 소유 외부 환경, scope 충돌 없이는 진행할 수 없으면 review stub의 `사용자 리뷰 요청`에 근거를 기록하고 중단한다. 구현 중 사용자에게 직접 묻거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 재실행으로 채울 수 있는 검증 증거 공백은 사용자 리뷰 요청이 아니다. + +## 배경 + +`op-pick`은 durable operation store가 생긴 뒤 worker가 queued operation을 중복 없이 running으로 가져가는 기준을 확정하는 작업이다. 현재 worker는 scaffold 로그만 남기며, operation store도 pick API를 갖고 있지 않다. + +## 사용자 리뷰 요청 흐름 + +구현 중 차단은 active `CODE_REVIEW-*-G??.md`의 `사용자 리뷰 요청` 섹션에 기록한다. 이 섹션은 `agent-ops/skills/common/_templates/implementation-user-review-request-section.md` 형식을 따른다. 직접 사용자 프롬프트는 금지되며, code-review가 요청 타당성 검증과 실제 `USER_REVIEW.md` 작성을 맡는다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- Task ids: + - `op-pick`: worker가 pending operation을 안전하게 pick하는 기준을 만든다. +- Completion mode: check-on-pass + +## 분석 결과 + +### 읽은 파일 + +- `agent-roadmap/current.md` +- `agent-roadmap/phase/control-plane-foundation/PHASE.md` +- `agent-roadmap/phase/control-plane-foundation/milestones/operation-event-outbox.md` +- `agent-ops/rules/project/domain/core/rules.md` +- `agent-test/local/rules.md` +- `agent-test/local/core-smoke.md` +- `services/core/go.mod` +- `services/core/cmd/worker/main.go` +- `services/core/internal/config/config.go` +- `services/core/internal/config/config_test.go` +- `services/core/internal/core/model.go` +- `services/core/internal/storage/storage.go` +- `services/core/internal/storage/postgres.go` +- `services/core/internal/storage/storage_test.go` +- `services/core/internal/storage/postgres_test.go` +- `services/core/internal/storage/postgres_repo_test.go` +- `services/core/internal/worker/runner.go` +- `services/core/migrations/00001_initial.sql` + +### 테스트 환경 규칙 + +- test_env: `local`. +- `agent-test/local/rules.md` present/read. Matching profile: `agent-test/local/core-smoke.md`. +- Applied command contract: core changes require `cd services/core && go test ./...`; final verification here uses fresh `go test -count=1 ./...`. +- No `<확인 필요>` command values were found. Postgres duplicate-pick integration test depends on `GITO_TEST_DATABASE_URL`; when unset it skips by existing pattern. + +### 테스트 커버리지 공백 + +- Worker runner has no test file today. Add `services/core/internal/worker/runner_test.go`. +- Pending operation pick has no store API or Postgres test today. Add integration coverage for concurrent picks. +- Command-level worker DB bootstrap is not covered; this plan keeps it out of scope unless a shared migration/bootstrap helper already exists after predecessor work. + +### 심볼 참조 + +- Planned `OperationStore` extension depends on `01_operation_store` replacing `SaveOperation`. Current references to operation store are limited to storage fakes and nil accessors. +- `NewRunner` is called by `services/core/cmd/worker/main.go`; keep the public constructor backward-compatible or update the call site in the same change. + +### 분할 판단 + +- Split policy evaluated before file selection. +- Shared task group: `agent-task/m-operation-event-outbox/`. +- `01_operation_store`: predecessor, must produce `agent-task/m-operation-event-outbox/01_operation_store/complete.log`. +- `02+01_operation_pick`: current dependent subtask. Predecessor index `01` is not satisfied yet; implementation must wait for the active predecessor to PASS. + +### 범위 결정 근거 + +- Exclude operation execution, git command dispatch, retries, and lease release after execution. +- Exclude durable outbox append/publish and proto-socket stream mapping. +- Exclude full worker daemon loop and backoff policy; this task only defines safe pick and one worker iteration boundary. +- Exclude Flutter/provider projection work. + +### 빌드 등급 + +- Build: `local-G06`. Review: `cloud-G06`. +- Rationale: bounded storage/worker code, but concurrent Postgres picking and worker boundary are easy to get subtly wrong. + +## 의존 관계 및 구현 순서 + +- This task directory is `02+01_operation_pick`; runtime dependency source of truth is predecessor `01`. +- Before implementation, require `agent-task/m-operation-event-outbox/01_operation_store/complete.log`. +- Do not implement this task against the pre-01 `OperationStore` shape. + +## 구현 체크리스트 + +- [ ] `OperationStore`에 pending pick API를 추가하고 `pgOperationStore`에서 `FOR UPDATE SKIP LOCKED` 기반으로 queued operation을 running으로 전환한다. +- [ ] worker runner에 one-iteration pick boundary를 추가하고 picker가 없거나 worker disabled일 때의 동작을 테스트한다. +- [ ] concurrent pick 테스트를 작성한다. 검증: 같은 operation이 중복 pick되지 않는다. +- [ ] `go test -count=1 ./internal/storage ./internal/worker`와 `go test -count=1 ./...`를 실행한다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +### [API-1] Atomic pending operation pick + +#### 문제 + +- [services/core/internal/storage/storage.go](/config/workspace/gito/services/core/internal/storage/storage.go:53)의 operation store 경계는 pending operation을 atomically claim하는 API를 갖고 있지 않다. +- [services/core/internal/storage/postgres.go](/config/workspace/gito/services/core/internal/storage/postgres.go:61)는 현재 operation store를 반환하지 않는다. 01 완료 후에는 실제 `pgOperationStore`가 존재해야 한다. + +Before: + +```go +53 type OperationStore interface { +54 SaveOperation(ctx context.Context, op core.Operation) error +55 GetOperation(ctx context.Context, id string) (core.Operation, error) +56 ListOperations(ctx context.Context, repoID string) ([]core.Operation, error) +57 } +``` + +#### 해결 방법 + +- 01 완료 후의 `OperationStore`에 `PickQueuedOperation(ctx context.Context, now time.Time) (core.Operation, bool, error)`를 추가한다. +- Postgres 구현은 아래 형태의 CTE를 사용한다. + +```sql +WITH candidate AS ( + SELECT id + FROM operations + WHERE state=$1 + ORDER BY created_at, id + FOR UPDATE SKIP LOCKED + LIMIT 1 +) +UPDATE operations AS op +SET state=$2, updated_at=$3 +FROM candidate +WHERE op.id=candidate.id +RETURNING ... +``` + +- no rows이면 `(core.Operation{}, false, nil)`을 반환한다. + +#### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/storage/storage.go`: pick method 추가. +- [ ] `services/core/internal/storage/postgres.go`: `pgOperationStore.PickQueuedOperation` 구현. +- [ ] `services/core/internal/storage/storage_test.go`: fakeOps compile check 갱신. +- [ ] `services/core/internal/storage/postgres_test.go`: concurrent duplicate-pick integration test 추가. + +#### 테스트 작성 + +- `TestPostgresOperationStorePickQueuedOperationDoesNotDuplicate`: 여러 goroutine이 동시에 pick해도 같은 operation id가 두 번 반환되지 않고 picked count가 seeded queued count를 넘지 않는다. +- no queued row이면 `ok=false`. + +#### 중간 검증 + +```bash +cd services/core && go test -count=1 ./internal/storage +``` + +Expected: storage package passes; DB-specific cases skip only when `GITO_TEST_DATABASE_URL` is unset. + +### [API-2] Worker run-one pick boundary + +#### 문제 + +- [services/core/internal/worker/runner.go](/config/workspace/gito/services/core/internal/worker/runner.go:18)의 `Run`은 enabled 상태에서도 scaffold 로그만 남기고 pending operation을 관찰하지 않는다. + +Before: + +```go +18 func (r *Runner) Run() error { +19 if !r.cfg.WorkerEnabled { +20 r.logger.Info("worker disabled") +21 return nil +22 } +23 r.logger.Info("worker scaffold ready") +24 return nil +25 } +``` + +#### 해결 방법 + +- worker package에 small interface를 둔다. + +```go +type OperationPicker interface { + PickQueuedOperation(ctx context.Context, now time.Time) (core.Operation, bool, error) +} +``` + +- `Runner`에 picker와 clock을 주입할 수 있게 한다. 기존 constructor 호환을 유지하거나 `cmd/worker/main.go`를 함께 갱신한다. +- `RunOnce(ctx)`를 추가해 disabled/no picker/no work/picked/error path를 명확히 한다. `Run()`은 이번 범위에서 `RunOnce(context.Background())`를 호출하는 one-iteration scaffold로 둔다. +- 실제 operation 실행, retry loop, sleep/backoff는 다음 작업으로 남긴다. + +#### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/worker/runner.go`: picker interface, `RunOnce`, logging/error path 구현. +- [ ] `services/core/internal/worker/runner_test.go`: fake picker 기반 단위 테스트 추가. +- [ ] `services/core/cmd/worker/main.go`: constructor signature가 바뀌면 call site만 맞춘다. DB bootstrap은 이번 계획 범위가 아니다. + +#### 테스트 작성 + +- `TestRunnerRunOnceSkipsWhenDisabled`: disabled config는 picker를 호출하지 않는다. +- `TestRunnerRunOnceReportsNoWork`: picker ok=false path는 nil error. +- `TestRunnerRunOncePicksOperation`: queued operation pick path는 nil error and fake picker count=1. +- `TestRunnerRunOnceReturnsPickerError`: picker error bubbles up. + +#### 중간 검증 + +```bash +cd services/core && go test -count=1 ./internal/worker +``` + +Expected: worker package passes. + +## 수정 파일 요약 + +| 파일 | 항목 | +| --- | --- | +| `services/core/internal/storage/storage.go` | API-1 | +| `services/core/internal/storage/postgres.go` | API-1 | +| `services/core/internal/storage/storage_test.go` | API-1 | +| `services/core/internal/storage/postgres_test.go` | API-1 | +| `services/core/internal/worker/runner.go` | API-2 | +| `services/core/internal/worker/runner_test.go` | API-2 | +| `services/core/cmd/worker/main.go` | API-2 | + +## 최종 검증 + +```bash +cd services/core && go test -count=1 ./internal/storage ./internal/worker +cd services/core && go test -count=1 ./... +``` + +Expected: all packages pass; Postgres integration tests skip only if `GITO_TEST_DATABASE_URL` is unset. + +모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/services/core/internal/core/model.go b/services/core/internal/core/model.go index 66d76e9..c395dc8 100644 --- a/services/core/internal/core/model.go +++ b/services/core/internal/core/model.go @@ -30,10 +30,11 @@ const ( ) type Operation struct { - ID string - RepoID string - Type OperationType - State OperationState + ID string + RepoID string + Type OperationType + State OperationState + // IdempotencyKey is an optional globally unique create-dedupe key. IdempotencyKey string CreatedBy string CreatedAt time.Time @@ -50,6 +51,15 @@ const ( OperationAgentRun OperationType = "agent_run" ) +func (t OperationType) Valid() bool { + switch t { + case OperationClone, OperationFetch, OperationCommit, OperationPush, OperationAgentRun: + return true + default: + return false + } +} + type OperationState string const ( @@ -60,6 +70,38 @@ const ( OperationCancelled OperationState = "cancelled" ) +func (s OperationState) Valid() bool { + switch s { + case OperationQueued, OperationRunning, OperationSucceeded, OperationFailed, OperationCancelled: + return true + default: + return false + } +} + +func (s OperationState) Terminal() bool { + switch s { + case OperationSucceeded, OperationFailed, OperationCancelled: + return true + default: + return false + } +} + +func (s OperationState) CanTransitionTo(next OperationState) bool { + if !s.Valid() || !next.Valid() || s == next { + return false + } + switch s { + case OperationQueued: + return next == OperationRunning || next == OperationCancelled + case OperationRunning: + return next == OperationSucceeded || next == OperationFailed || next == OperationCancelled + default: + return false + } +} + type RevisionEvent struct { RepoID string Branch string diff --git a/services/core/internal/core/model_test.go b/services/core/internal/core/model_test.go new file mode 100644 index 0000000..e57a4b9 --- /dev/null +++ b/services/core/internal/core/model_test.go @@ -0,0 +1,62 @@ +package core + +import "testing" + +func TestOperationTypeValid(t *testing.T) { + valid := []OperationType{ + OperationClone, + OperationFetch, + OperationCommit, + OperationPush, + OperationAgentRun, + } + for _, typ := range valid { + if !typ.Valid() { + t.Fatalf("operation type %q should be valid", typ) + } + } + if OperationType("unknown").Valid() { + t.Fatal("unknown operation type should be invalid") + } +} + +func TestOperationStateValidTerminalAndTransitions(t *testing.T) { + tests := []struct { + name string + from OperationState + to OperationState + allowed bool + terminal bool + }{ + {name: "queued starts running", from: OperationQueued, to: OperationRunning, allowed: true}, + {name: "queued cancels", from: OperationQueued, to: OperationCancelled, allowed: true}, + {name: "running succeeds", from: OperationRunning, to: OperationSucceeded, allowed: true}, + {name: "running fails", from: OperationRunning, to: OperationFailed, allowed: true}, + {name: "running cancels", from: OperationRunning, to: OperationCancelled, allowed: true}, + {name: "queued cannot succeed directly", from: OperationQueued, to: OperationSucceeded}, + {name: "running cannot go back to queued", from: OperationRunning, to: OperationQueued}, + {name: "succeeded is terminal", from: OperationSucceeded, to: OperationRunning, terminal: true}, + {name: "failed is terminal", from: OperationFailed, to: OperationRunning, terminal: true}, + {name: "cancelled is terminal", from: OperationCancelled, to: OperationRunning, terminal: true}, + {name: "same state is not a transition", from: OperationQueued, to: OperationQueued}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if !tt.from.Valid() { + t.Fatalf("from state %q should be valid", tt.from) + } + if !tt.to.Valid() { + t.Fatalf("to state %q should be valid", tt.to) + } + if got := tt.from.CanTransitionTo(tt.to); got != tt.allowed { + t.Fatalf("CanTransitionTo: got %v want %v", got, tt.allowed) + } + if got := tt.from.Terminal(); got != tt.terminal { + t.Fatalf("Terminal: got %v want %v", got, tt.terminal) + } + }) + } + if OperationState("unknown").Valid() { + t.Fatal("unknown operation state should be invalid") + } +} diff --git a/services/core/internal/storage/postgres.go b/services/core/internal/storage/postgres.go index 318c7b3..18ab0f2 100644 --- a/services/core/internal/storage/postgres.go +++ b/services/core/internal/storage/postgres.go @@ -20,6 +20,7 @@ type PgStore struct { pool *pgxpool.Pool repos *pgRepoStore leases *pgWorkspaceLeaseStore + ops *pgOperationStore watches *pgBranchWatchStore cursors *pgRevisionCursorStore deliveries *pgProviderDeliveryStore @@ -46,6 +47,7 @@ func NewPgStore(ctx context.Context, dsn string, migrationSQL string) (*PgStore, s := &PgStore{pool: pool} s.repos = &pgRepoStore{pool: pool} s.leases = &pgWorkspaceLeaseStore{pool: pool} + s.ops = &pgOperationStore{pool: pool} s.watches = &pgBranchWatchStore{pool: pool} s.cursors = &pgRevisionCursorStore{pool: pool} s.deliveries = &pgProviderDeliveryStore{pool: pool} @@ -58,7 +60,7 @@ func (s *PgStore) Ping(ctx context.Context) error { return s.pool.Ping(ctx) } func (s *PgStore) Repos() RepoStore { return s.repos } func (s *PgStore) WorkspaceLeases() WorkspaceLeaseStore { return s.leases } -func (s *PgStore) Operations() OperationStore { return nil } +func (s *PgStore) Operations() OperationStore { return s.ops } func (s *PgStore) OperationEvents() OperationEventStore { return nil } func (s *PgStore) BranchWatches() BranchWatchStore { return s.watches } func (s *PgStore) RevisionCursors() RevisionCursorStore { return s.cursors } @@ -497,6 +499,236 @@ func nullableTime(value *time.Time) any { return value.UTC() } +// pgOperationStore + +type pgOperationStore struct{ pool *pgxpool.Pool } + +const opColumns = "id, repo_id, type, state, idempotency_key, created_by, created_at, updated_at" + +// opIdempotencyConstraint is the partial unique index on operations.idempotency_key +// (see migrations/00001_initial.sql). A unique violation naming this constraint is +// an idempotent retry; any other unique violation is a duplicate create. +const opIdempotencyConstraint = "ux_operations_idempotency" + +func (s *pgOperationStore) CreateOperation(ctx context.Context, op core.Operation) (OperationCreateResult, error) { + op, err := normalizeOperation(op) + if err != nil { + return OperationCreateResult{}, err + } + + const q = ` + INSERT INTO operations (id, repo_id, type, state, idempotency_key, created_by) + VALUES ($1, $2, $3, $4, $5, $6) + RETURNING ` + opColumns + created, err := scanOperation(s.pool.QueryRow(ctx, q, + op.ID, + op.RepoID, + op.Type, + op.State, + nullableString(op.IdempotencyKey), + op.CreatedBy, + )) + if err != nil { + if constraint, ok := uniqueViolationConstraint(err); ok { + // Only the idempotency index conflict represents an idempotent retry + // and returns the existing operation. Any other unique violation + // (id primary key, etc.) is a genuine duplicate create, regardless of + // whether an idempotency key was supplied. + if constraint == opIdempotencyConstraint && op.IdempotencyKey != "" { + existing, getErr := s.getByIdempotencyKey(ctx, op.IdempotencyKey) + if getErr != nil { + return OperationCreateResult{}, getErr + } + return OperationCreateResult{Operation: existing, Created: false}, nil + } + return OperationCreateResult{}, fmt.Errorf("%w: %s", ErrOperationAlreadyExists, op.ID) + } + return OperationCreateResult{}, fmt.Errorf("create operation: %w", err) + } + return OperationCreateResult{Operation: created, Created: true}, nil +} + +func (s *pgOperationStore) getByIdempotencyKey(ctx context.Context, key string) (core.Operation, error) { + const q = `SELECT ` + opColumns + ` FROM operations WHERE idempotency_key=$1` + op, err := scanOperation(s.pool.QueryRow(ctx, q, key)) + if err != nil { + if isNoRows(err) { + return core.Operation{}, fmt.Errorf("%w: idempotency_key %s", ErrOperationNotFound, key) + } + return core.Operation{}, fmt.Errorf("get operation by idempotency key: %w", err) + } + return op, nil +} + +func (s *pgOperationStore) GetOperation(ctx context.Context, id string) (core.Operation, error) { + id = strings.TrimSpace(id) + if id == "" { + return core.Operation{}, fmt.Errorf("%w: id is required", ErrInvalidOperation) + } + + const q = `SELECT ` + opColumns + ` FROM operations WHERE id=$1` + op, err := scanOperation(s.pool.QueryRow(ctx, q, id)) + if err != nil { + if isNoRows(err) { + return core.Operation{}, fmt.Errorf("%w: %s", ErrOperationNotFound, id) + } + return core.Operation{}, fmt.Errorf("get operation: %w", err) + } + return op, nil +} + +func (s *pgOperationStore) ListOperations(ctx context.Context, repoID string) ([]core.Operation, error) { + repoID = strings.TrimSpace(repoID) + if repoID == "" { + return nil, fmt.Errorf("%w: repo_id is required", ErrInvalidOperation) + } + + const q = `SELECT ` + opColumns + ` FROM operations WHERE repo_id=$1 ORDER BY created_at, id` + rows, err := s.pool.Query(ctx, q, repoID) + if err != nil { + return nil, fmt.Errorf("list operations: %w", err) + } + defer rows.Close() + + var out []core.Operation + for rows.Next() { + op, err := scanOperation(rows) + if err != nil { + return nil, fmt.Errorf("scan operation: %w", err) + } + out = append(out, op) + } + return out, rows.Err() +} + +func (s *pgOperationStore) StartOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) { + return s.transition(ctx, id, core.OperationRunning, now) +} + +func (s *pgOperationStore) SucceedOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) { + return s.transition(ctx, id, core.OperationSucceeded, now) +} + +func (s *pgOperationStore) FailOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) { + return s.transition(ctx, id, core.OperationFailed, now) +} + +func (s *pgOperationStore) CancelOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) { + return s.transition(ctx, id, core.OperationCancelled, now) +} + +func (s *pgOperationStore) PickQueuedOperation(ctx context.Context, now time.Time) (core.Operation, bool, error) { + const q = ` + WITH candidate AS ( + SELECT id + FROM operations + WHERE state=$1 + ORDER BY created_at, id + FOR UPDATE SKIP LOCKED + LIMIT 1 + ) + UPDATE operations AS op + SET state=$2, updated_at=$3 + FROM candidate + WHERE op.id=candidate.id + RETURNING ` + opColumns + + op, err := scanOperation(s.pool.QueryRow(ctx, q, core.OperationQueued, core.OperationRunning, now.UTC())) + if err != nil { + if isNoRows(err) { + return core.Operation{}, false, nil + } + return core.Operation{}, false, fmt.Errorf("pick queued operation: %w", err) + } + return op, true, nil +} + +// transition reads the current operation, validates the state change against +// the core transition rules, and persists the next state with an optimistic +// guard on the observed current state. +func (s *pgOperationStore) transition(ctx context.Context, id string, next core.OperationState, now time.Time) (core.Operation, error) { + id = strings.TrimSpace(id) + if id == "" { + return core.Operation{}, fmt.Errorf("%w: id is required", ErrInvalidOperation) + } + + current, err := s.GetOperation(ctx, id) + if err != nil { + return core.Operation{}, err + } + if !current.State.CanTransitionTo(next) { + return core.Operation{}, fmt.Errorf("%w: cannot transition %s from %s to %s", ErrInvalidOperation, id, current.State, next) + } + + const q = ` + UPDATE operations + SET state=$3, + updated_at=$4 + WHERE id=$1 AND state=$2 + RETURNING ` + opColumns + op, err := scanOperation(s.pool.QueryRow(ctx, q, id, current.State, next, now.UTC())) + if err != nil { + if isNoRows(err) { + // State changed between read and update; surface as an invalid transition. + return core.Operation{}, fmt.Errorf("%w: %s no longer in state %s", ErrInvalidOperation, id, current.State) + } + return core.Operation{}, fmt.Errorf("transition operation: %w", err) + } + return op, nil +} + +func normalizeOperation(op core.Operation) (core.Operation, error) { + op.ID = strings.TrimSpace(op.ID) + op.RepoID = strings.TrimSpace(op.RepoID) + op.IdempotencyKey = strings.TrimSpace(op.IdempotencyKey) + op.CreatedBy = strings.TrimSpace(op.CreatedBy) + + var missing []string + if op.ID == "" { + missing = append(missing, "id") + } + if op.RepoID == "" { + missing = append(missing, "repo_id") + } + if op.CreatedBy == "" { + missing = append(missing, "created_by") + } + if len(missing) > 0 { + return core.Operation{}, fmt.Errorf("%w: required fields missing: %s", ErrInvalidOperation, strings.Join(missing, ", ")) + } + if !op.Type.Valid() { + return core.Operation{}, fmt.Errorf("%w: unsupported type %q", ErrInvalidOperation, op.Type) + } + if op.State == "" { + op.State = core.OperationQueued + } + if op.State != core.OperationQueued { + return core.Operation{}, fmt.Errorf("%w: create state must be %q, got %q", ErrInvalidOperation, core.OperationQueued, op.State) + } + return op, nil +} + +func scanOperation(scanner repoScanner) (core.Operation, error) { + var op core.Operation + var idempotencyKey sql.NullString + if err := scanner.Scan( + &op.ID, + &op.RepoID, + &op.Type, + &op.State, + &idempotencyKey, + &op.CreatedBy, + &op.CreatedAt, + &op.UpdatedAt, + ); err != nil { + return core.Operation{}, err + } + if idempotencyKey.Valid { + op.IdempotencyKey = idempotencyKey.String + } + return op, nil +} + // pgBranchWatchStore type pgBranchWatchStore struct{ pool *pgxpool.Pool } @@ -621,6 +853,18 @@ func isNoRows(err error) bool { } func isUniqueViolation(err error) bool { - var pgErr *pgconn.PgError - return errors.As(err, &pgErr) && pgErr.Code == "23505" + _, ok := uniqueViolationConstraint(err) + return ok +} + +// uniqueViolationConstraint reports whether err is a Postgres unique violation +// (SQLSTATE 23505) and returns the violated constraint/index name when so. The +// name lets callers distinguish, for example, an idempotency-key index conflict +// from a primary-key collision. +func uniqueViolationConstraint(err error) (string, bool) { + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) && pgErr.Code == "23505" { + return pgErr.ConstraintName, true + } + return "", false } diff --git a/services/core/internal/storage/postgres_repo_test.go b/services/core/internal/storage/postgres_repo_test.go index f788bd5..2adac48 100644 --- a/services/core/internal/storage/postgres_repo_test.go +++ b/services/core/internal/storage/postgres_repo_test.go @@ -6,6 +6,8 @@ import ( "testing" "time" + "github.com/jackc/pgx/v5/pgconn" + "git.toki-labs.com/toki/gito/services/core/internal/core" ) @@ -50,6 +52,109 @@ func TestPgWorkspaceLeaseStoreValidatesBeforeDatabase(t *testing.T) { } } +func TestPgOperationStoreValidatesBeforeDatabase(t *testing.T) { + store := &pgOperationStore{} + base := core.Operation{ + ID: "op-1", + RepoID: "repo-1", + Type: core.OperationClone, + State: core.OperationQueued, + CreatedBy: "agent-shell", + } + + tests := []struct { + name string + mutate func(op core.Operation) core.Operation + }{ + { + name: "missing id", + mutate: func(op core.Operation) core.Operation { op.ID = " "; return op }, + }, + { + name: "missing repo_id", + mutate: func(op core.Operation) core.Operation { op.RepoID = ""; return op }, + }, + { + name: "missing created_by", + mutate: func(op core.Operation) core.Operation { op.CreatedBy = ""; return op }, + }, + { + name: "invalid type", + mutate: func(op core.Operation) core.Operation { op.Type = core.OperationType("merge"); return op }, + }, + { + name: "non-queued create state", + mutate: func(op core.Operation) core.Operation { op.State = core.OperationRunning; return op }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := store.CreateOperation(context.Background(), tt.mutate(base)) + if !errors.Is(err, ErrInvalidOperation) { + t.Fatalf("CreateOperation %s: got %v want %v", tt.name, err, ErrInvalidOperation) + } + }) + } + + if _, err := store.GetOperation(context.Background(), " "); !errors.Is(err, ErrInvalidOperation) { + t.Fatalf("GetOperation empty id: got %v want %v", err, ErrInvalidOperation) + } + if _, err := store.ListOperations(context.Background(), ""); !errors.Is(err, ErrInvalidOperation) { + t.Fatalf("ListOperations empty repo: got %v want %v", err, ErrInvalidOperation) + } + if _, err := store.StartOperation(context.Background(), "", time.Now()); !errors.Is(err, ErrInvalidOperation) { + t.Fatalf("StartOperation empty id: got %v want %v", err, ErrInvalidOperation) + } +} + +func TestUniqueViolationConstraintRouting(t *testing.T) { + tests := []struct { + name string + err error + wantUnique bool + wantConstraint string + }{ + { + name: "idempotency index conflict", + err: &pgconn.PgError{Code: "23505", ConstraintName: opIdempotencyConstraint}, + wantUnique: true, + wantConstraint: opIdempotencyConstraint, + }, + { + name: "primary key collision", + err: &pgconn.PgError{Code: "23505", ConstraintName: "operations_pkey"}, + wantUnique: true, + wantConstraint: "operations_pkey", + }, + { + name: "non-unique pg error", + err: &pgconn.PgError{Code: "23503", ConstraintName: "operations_repo_id_fkey"}, + wantUnique: false, + }, + { + name: "non-pg error", + err: errors.New("connection reset"), + wantUnique: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + constraint, unique := uniqueViolationConstraint(tt.err) + if unique != tt.wantUnique { + t.Fatalf("uniqueViolationConstraint unique: got %v want %v", unique, tt.wantUnique) + } + if unique && constraint != tt.wantConstraint { + t.Fatalf("uniqueViolationConstraint name: got %q want %q", constraint, tt.wantConstraint) + } + if isUniqueViolation(tt.err) != tt.wantUnique { + t.Fatalf("isUniqueViolation: got %v want %v", isUniqueViolation(tt.err), tt.wantUnique) + } + }) + } +} + func TestLeaseStateForReleaseDerivesStateFromOperationResult(t *testing.T) { tests := []struct { name string diff --git a/services/core/internal/storage/postgres_test.go b/services/core/internal/storage/postgres_test.go index 8f34290..a2528e5 100644 --- a/services/core/internal/storage/postgres_test.go +++ b/services/core/internal/storage/postgres_test.go @@ -418,6 +418,156 @@ func TestPostgresMigrationDropsLegacyRevisionCursorRepoFK(t *testing.T) { } } +func TestPostgresOperationStoreLifecycleAndIdempotency(t *testing.T) { + dsn := os.Getenv("GITO_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("GITO_TEST_DATABASE_URL not set; skipping Postgres integration test") + } + + ctx := context.Background() + dsn = isolatedPostgresDSN(t, ctx, dsn) + + migrationData, err := os.ReadFile("../../migrations/00001_initial.sql") + if err != nil { + t.Fatalf("read migration: %v", err) + } + + store, err := storage.NewPgStore(ctx, dsn, string(migrationData)) + if err != nil { + t.Fatalf("open store: %v", err) + } + defer store.Close() + + if store.Operations() == nil { + t.Fatal("Operations() must not return nil") + } + + repo := core.Repo{ + ID: "op-repo", + Name: "Op Repo", + RemoteURL: "ssh://git.example.invalid/toki/op-repo.git", + DefaultBranch: "main", + WorkspaceRoot: "/workspaces/op-repo", + } + if err := store.Repos().CreateRepo(ctx, repo); err != nil { + t.Fatalf("create repo: %v", err) + } + + ops := store.Operations() + + // Create with idempotency key. + op := core.Operation{ + ID: "op-001", + RepoID: repo.ID, + Type: core.OperationClone, + State: core.OperationQueued, + IdempotencyKey: "idem-clone-1", + CreatedBy: "agent-shell", + } + created, err := ops.CreateOperation(ctx, op) + if err != nil { + t.Fatalf("create operation: %v", err) + } + if !created.Created { + t.Fatal("first create must report Created=true") + } + if created.Operation.State != core.OperationQueued { + t.Fatalf("created state: got %q want %q", created.Operation.State, core.OperationQueued) + } + + // Duplicate idempotency key (different id) returns the existing operation. + dup := op + dup.ID = "op-001-retry" + dupResult, err := ops.CreateOperation(ctx, dup) + if err != nil { + t.Fatalf("duplicate idempotency create: %v", err) + } + if dupResult.Created { + t.Fatal("duplicate idempotency create must report Created=false") + } + if dupResult.Operation.ID != op.ID { + t.Fatalf("duplicate idempotency returned id %q want %q", dupResult.Operation.ID, op.ID) + } + + // id collision without idempotency key is a hard duplicate. + noKey := core.Operation{ + ID: "op-002", + RepoID: repo.ID, + Type: core.OperationFetch, + State: core.OperationQueued, + CreatedBy: "agent-shell", + } + if _, err := ops.CreateOperation(ctx, noKey); err != nil { + t.Fatalf("create no-key operation: %v", err) + } + if _, err := ops.CreateOperation(ctx, noKey); !errors.Is(err, storage.ErrOperationAlreadyExists) { + t.Fatalf("id collision error: got %v want %v", err, storage.ErrOperationAlreadyExists) + } + + // Same id as an existing operation but a different idempotency key collides on + // the primary key, not the idempotency index, so it is a duplicate create. + sameIDOtherKey := op + sameIDOtherKey.IdempotencyKey = "idem-clone-2" + if _, err := ops.CreateOperation(ctx, sameIDOtherKey); !errors.Is(err, storage.ErrOperationAlreadyExists) { + t.Fatalf("same id different idempotency key error: got %v want %v", err, storage.ErrOperationAlreadyExists) + } + + // queued -> running -> succeeded. + now := time.Now().UTC() + running, err := ops.StartOperation(ctx, op.ID, now) + if err != nil { + t.Fatalf("start operation: %v", err) + } + if running.State != core.OperationRunning { + t.Fatalf("started state: got %q want %q", running.State, core.OperationRunning) + } + succeeded, err := ops.SucceedOperation(ctx, op.ID, now.Add(time.Second)) + if err != nil { + t.Fatalf("succeed operation: %v", err) + } + if succeeded.State != core.OperationSucceeded { + t.Fatalf("succeeded state: got %q want %q", succeeded.State, core.OperationSucceeded) + } + + // succeeded -> failed is an invalid transition. + if _, err := ops.FailOperation(ctx, op.ID, now.Add(2*time.Second)); !errors.Is(err, storage.ErrInvalidOperation) { + t.Fatalf("invalid transition error: got %v want %v", err, storage.ErrInvalidOperation) + } + + // starting an already-terminal operation is also invalid. + if _, err := ops.StartOperation(ctx, op.ID, now); !errors.Is(err, storage.ErrInvalidOperation) { + t.Fatalf("start terminal error: got %v want %v", err, storage.ErrInvalidOperation) + } + + // GetOperation and not-found path. + got, err := ops.GetOperation(ctx, op.ID) + if err != nil { + t.Fatalf("get operation: %v", err) + } + if got.State != core.OperationSucceeded { + t.Fatalf("get operation state: got %q want %q", got.State, core.OperationSucceeded) + } + if _, err := ops.GetOperation(ctx, "missing"); !errors.Is(err, storage.ErrOperationNotFound) { + t.Fatalf("get missing error: got %v want %v", err, storage.ErrOperationNotFound) + } + + // List ordering is stable (created_at, id). + listed, err := ops.ListOperations(ctx, repo.ID) + if err != nil { + t.Fatalf("list operations: %v", err) + } + if len(listed) != 2 { + t.Fatalf("listed operations: got %d want 2", len(listed)) + } + for i := 1; i < len(listed); i++ { + prev, cur := listed[i-1], listed[i] + if prev.CreatedAt.After(cur.CreatedAt) || + (prev.CreatedAt.Equal(cur.CreatedAt) && prev.ID > cur.ID) { + t.Fatalf("list ordering unstable at %d: %+v then %+v", i, prev, cur) + } + } +} + func isolatedPostgresDSN(t *testing.T, ctx context.Context, dsn string) string { t.Helper() diff --git a/services/core/internal/storage/storage.go b/services/core/internal/storage/storage.go index c4ea6a8..d880054 100644 --- a/services/core/internal/storage/storage.go +++ b/services/core/internal/storage/storage.go @@ -29,12 +29,15 @@ type RepoStore interface { } var ( - ErrInvalidRepo = errors.New("invalid repo") - ErrRepoAlreadyExists = errors.New("repo already exists") - ErrRepoNotFound = errors.New("repo not found") - ErrInvalidLease = errors.New("invalid workspace lease") - ErrLeaseAlreadyExists = errors.New("workspace lease already exists") - ErrLeaseNotFound = errors.New("workspace lease not found") + ErrInvalidRepo = errors.New("invalid repo") + ErrRepoAlreadyExists = errors.New("repo already exists") + ErrRepoNotFound = errors.New("repo not found") + ErrInvalidLease = errors.New("invalid workspace lease") + ErrLeaseAlreadyExists = errors.New("workspace lease already exists") + ErrLeaseNotFound = errors.New("workspace lease not found") + ErrInvalidOperation = errors.New("invalid operation") + ErrOperationAlreadyExists = errors.New("operation already exists") + ErrOperationNotFound = errors.New("operation not found") ) type WorkspaceLeaseStore interface { @@ -50,10 +53,23 @@ type LeaseRelease struct { Dirty bool } +// OperationCreateResult is returned by CreateOperation. +// Created is true when this call inserted a new operation; it is false when an +// existing operation was returned because the idempotency key already existed. +type OperationCreateResult struct { + Operation core.Operation + Created bool +} + type OperationStore interface { - SaveOperation(ctx context.Context, op core.Operation) error + CreateOperation(ctx context.Context, op core.Operation) (OperationCreateResult, error) GetOperation(ctx context.Context, id string) (core.Operation, error) ListOperations(ctx context.Context, repoID string) ([]core.Operation, error) + StartOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) + SucceedOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) + FailOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) + CancelOperation(ctx context.Context, id string, now time.Time) (core.Operation, error) + PickQueuedOperation(ctx context.Context, now time.Time) (core.Operation, bool, error) } type OperationEvent struct { diff --git a/services/core/internal/storage/storage_test.go b/services/core/internal/storage/storage_test.go index 97a906f..7a9e966 100644 --- a/services/core/internal/storage/storage_test.go +++ b/services/core/internal/storage/storage_test.go @@ -49,13 +49,27 @@ var _ storage.WorkspaceLeaseStore = (*fakeLeases)(nil) type fakeOps struct{} -func (f *fakeOps) SaveOperation(_ context.Context, _ core.Operation) error { return nil } +func (f *fakeOps) CreateOperation(_ context.Context, op core.Operation) (storage.OperationCreateResult, error) { + return storage.OperationCreateResult{Operation: op, Created: true}, nil +} func (f *fakeOps) GetOperation(_ context.Context, _ string) (core.Operation, error) { return core.Operation{}, nil } func (f *fakeOps) ListOperations(_ context.Context, _ string) ([]core.Operation, error) { return nil, nil } +func (f *fakeOps) StartOperation(_ context.Context, _ string, _ time.Time) (core.Operation, error) { + return core.Operation{}, nil +} +func (f *fakeOps) SucceedOperation(_ context.Context, _ string, _ time.Time) (core.Operation, error) { + return core.Operation{}, nil +} +func (f *fakeOps) FailOperation(_ context.Context, _ string, _ time.Time) (core.Operation, error) { + return core.Operation{}, nil +} +func (f *fakeOps) CancelOperation(_ context.Context, _ string, _ time.Time) (core.Operation, error) { + return core.Operation{}, nil +} var _ storage.OperationStore = (*fakeOps)(nil) @@ -185,6 +199,9 @@ func TestInitialMigrationSupportsStoreBoundary(t *testing.T) { "state TEXT NOT NULL", "expires_at TIMESTAMPTZ", "UNIQUE (repo_id, slot)", + "type TEXT NOT NULL CHECK (type IN ('clone', 'fetch', 'commit', 'push', 'agent_run'))", + "state TEXT NOT NULL CHECK (state IN ('queued', 'running', 'succeeded', 'failed', 'cancelled'))", + "idempotency_key TEXT CHECK (idempotency_key IS NULL OR btrim(idempotency_key) <> '')", "operation_id", "ALTER TABLE revision_cursors DROP CONSTRAINT IF EXISTS revision_cursors_repo_id_fkey", "UNIQUE (provider, repo_id, branch)", diff --git a/services/core/migrations/00001_initial.sql b/services/core/migrations/00001_initial.sql index e4ee3cb..553d493 100644 --- a/services/core/migrations/00001_initial.sql +++ b/services/core/migrations/00001_initial.sql @@ -26,10 +26,10 @@ CREATE TABLE IF NOT EXISTS workspace_leases ( CREATE TABLE IF NOT EXISTS operations ( id TEXT PRIMARY KEY, repo_id TEXT NOT NULL REFERENCES repos(id), - type TEXT NOT NULL, - state TEXT NOT NULL, - idempotency_key TEXT, - created_by TEXT NOT NULL, + type TEXT NOT NULL CHECK (type IN ('clone', 'fetch', 'commit', 'push', 'agent_run')), + state TEXT NOT NULL CHECK (state IN ('queued', 'running', 'succeeded', 'failed', 'cancelled')), + idempotency_key TEXT CHECK (idempotency_key IS NULL OR btrim(idempotency_key) <> ''), + created_by TEXT NOT NULL CHECK (btrim(created_by) <> ''), created_at TIMESTAMPTZ NOT NULL DEFAULT now(), updated_at TIMESTAMPTZ NOT NULL DEFAULT now() );