From fad5627ae66be2b78afa890c87dcd22b319bb615 Mon Sep 17 00:00:00 2001 From: toki Date: Sun, 14 Jun 2026 04:08:15 +0900 Subject: [PATCH] =?UTF-8?q?feat:=20milestone=20work=20item=20creation/sync?= =?UTF-8?q?=20=EA=B5=AC=ED=98=84=20=EB=B0=8F=20roadmap=20sync=20=EA=B8=B0?= =?UTF-8?q?=EB=8A=A5=20=EC=B6=94=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Plane Todo projection 및 idempotency/retry 메커니즘 구현 - Roadmap sync identity 및 step 모델/DB 마이그레이션/쿼리 추가 - OpenAI 및 Plane adapter 개선 - roadmaps sync package: plane_format, plane_projection, retry 모듈 추가 - Storage layer: roadmap_sync_identities, roadmap_sync_steps 추가 - agent-task 아카이브 정리 --- .../milestone-work-item-creation-sync.md | 16 +- .../code_review_cloud_G07_0.log | 141 ++++++++++ .../04+03_plane_todo_projection/complete.log | 45 ++++ .../plan_cloud_G07_0.log} | 0 .../code_review_cloud_G07_0.log | 152 +++++++++++ .../code_review_cloud_G07_1.log | 194 ++++++++++++++ .../05+03,04_idempotency_retry/complete.log | 47 ++++ .../plan_cloud_G07_0.log} | 0 .../plan_cloud_G07_1.log | 94 +++++++ .../CODE_REVIEW-cloud-G07.md | 92 ------- .../CODE_REVIEW-cloud-G07.md | 92 ------- .../core/internal/adapters/openai/client.go | 4 +- .../internal/adapters/openai/client_test.go | 19 +- .../core/internal/adapters/plane/client.go | 53 +++- .../internal/adapters/plane/client_test.go | 96 +++++++ services/core/internal/db/models.go | 23 ++ .../db/roadmap_sync_identities.sql.go | 138 ++++++++++ .../internal/db/roadmap_sync_steps.sql.go | 67 +++++ services/core/internal/model/model.go | 8 +- .../core/internal/roadmapsync/identity.go | 24 ++ .../internal/roadmapsync/identity_test.go | 30 +++ .../core/internal/roadmapsync/plane_format.go | 148 +++++++++++ .../internal/roadmapsync/plane_format_test.go | 73 +++++ .../internal/roadmapsync/plane_projection.go | 102 +++++++ .../roadmapsync/plane_projection_test.go | 127 +++++++++ services/core/internal/roadmapsync/retry.go | 173 ++++++++++++ .../core/internal/roadmapsync/retry_test.go | 184 +++++++++++++ .../storage/roadmap_sync_identities.go | 84 ++++++ .../storage/roadmap_sync_identities_test.go | 249 ++++++++++++++++++ .../internal/storage/roadmap_sync_steps.go | 72 +++++ .../storage/roadmap_sync_steps_test.go | 180 +++++++++++++ services/core/internal/workitem/provider.go | 14 + .../00006_create_roadmap_sync_identities.sql | 34 +++ .../00007_create_roadmap_sync_steps.sql | 21 ++ .../core/queries/roadmap_sync_identities.sql | 25 ++ services/core/queries/roadmap_sync_steps.sql | 12 + 36 files changed, 2620 insertions(+), 213 deletions(-) create mode 100644 agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/code_review_cloud_G07_0.log create mode 100644 agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/complete.log rename agent-task/{m-milestone-work-item-creation-sync/04+03_plane_todo_projection/PLAN-cloud-G07.md => archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/plan_cloud_G07_0.log} (100%) create mode 100644 agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_0.log create mode 100644 agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_1.log create mode 100644 agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/complete.log rename agent-task/{m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/PLAN-cloud-G07.md => archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_0.log} (100%) create mode 100644 agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_1.log delete mode 100644 agent-task/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/CODE_REVIEW-cloud-G07.md delete mode 100644 agent-task/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/CODE_REVIEW-cloud-G07.md create mode 100644 services/core/internal/db/roadmap_sync_identities.sql.go create mode 100644 services/core/internal/db/roadmap_sync_steps.sql.go create mode 100644 services/core/internal/roadmapsync/plane_format.go create mode 100644 services/core/internal/roadmapsync/plane_format_test.go create mode 100644 services/core/internal/roadmapsync/plane_projection.go create mode 100644 services/core/internal/roadmapsync/plane_projection_test.go create mode 100644 services/core/internal/roadmapsync/retry.go create mode 100644 services/core/internal/roadmapsync/retry_test.go create mode 100644 services/core/internal/storage/roadmap_sync_identities.go create mode 100644 services/core/internal/storage/roadmap_sync_identities_test.go create mode 100644 services/core/internal/storage/roadmap_sync_steps.go create mode 100644 services/core/internal/storage/roadmap_sync_steps_test.go create mode 100644 services/core/migrations/00006_create_roadmap_sync_identities.sql create mode 100644 services/core/migrations/00007_create_roadmap_sync_steps.sql create mode 100644 services/core/queries/roadmap_sync_identities.sql create mode 100644 services/core/queries/roadmap_sync_steps.sql diff --git a/agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md b/agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md index 20d5f2e..39dfac8 100644 --- a/agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md +++ b/agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md @@ -50,11 +50,11 @@ Plane-origin 경로로 Milestone을 생성하고, `develop` branch의 agent-road Plane `Backlog + AGENT assignee` 상위 티켓 하나가 NomadCode project sync와 IOP Edge HTTP workspace agent 실행을 거쳐 `develop` source-of-truth에 Milestone 산출물을 만들고, 다시 Plane `Todo` 검토 상태로 투영되는 한 사이클을 최상위 실행 흐름으로 둔다. - [x] [cycle-project-config] Plane project target, git remote, source-of-truth branch, workspace base/repo 설정을 project sync 기준으로 해석할 수 있다. 검증: `[project-config]` Epic의 완료 근거와 `workitempipeline` project binding/slot metadata 연결로 확인했다. -- [ ] [cycle-workspace-topology] project workspace root 아래 `branches/main`, `branches/develop`, `slots/000+` 구조를 준비하고 branch workspace와 agent slot 상태를 분리한다. 검증: `branches/develop`은 source-of-truth scan 기준, `branches/main`은 release/promote 기준, `slots/`는 authoring 쓰기 작업 기준으로만 사용된다. -- [ ] [cycle-iop-http-ready] IOP Edge OpenAI-compatible HTTP(`/v1/responses`) workspace agent 실행 통로를 선행 조건으로 확인한다. 검증: NomadCode가 IOP CLI를 직접 실행하지 않고, `model`, `input`, `metadata.workspace` 요청만으로 workspace-bound agent target을 실행할 수 있다. -- [ ] [cycle-authoring-run] Plane 티켓 제목/본문과 provider work item identity를 입력으로 `slots/` workspace에서 1회 authoring run을 실행한다. 검증: agent가 slot 안에서 Milestone 파일을 작성/갱신하고 `develop`에 commit/push한다. -- [ ] [cycle-develop-sync] sync layer가 `branches/develop` 또는 remote `develop`에서 pushed Milestone을 감지하고 원래 Plane 티켓 identity와 매칭한다. 검증: slot local 변경이나 push 실패 상태만으로 Plane `Todo` projection을 수행하지 않는다. -- [ ] [cycle-plane-todo] 원본 Plane 본문을 `사용자 요청:` 댓글로 보존하고, `develop`의 Milestone 내용으로 Plane 본문/제목을 갱신한 뒤 `Todo`로 이동한다. 검증: 사용자에게 Plane Todo에서 `develop` 반영 결과를 검토할 수 있는 상태가 된다. +- [x] [cycle-workspace-topology] project workspace root 아래 `branches/main`, `branches/develop`, `slots/000+` 구조를 준비하고 branch workspace와 agent slot 상태를 분리한다. 검증: `branches/develop`은 source-of-truth scan 기준, `branches/main`은 release/promote 기준, `slots/`는 authoring 쓰기 작업 기준으로만 사용된다. +- [x] [cycle-iop-http-ready] IOP Edge OpenAI-compatible HTTP(`/v1/responses`) workspace agent 실행 통로를 선행 조건으로 확인한다. 검증: NomadCode가 IOP CLI를 직접 실행하지 않고, `model`, `input`, `metadata.workspace` 요청만으로 workspace-bound agent target을 실행할 수 있다. +- [x] [cycle-authoring-run] Plane 티켓 제목/본문과 provider work item identity를 입력으로 `slots/` workspace에서 1회 authoring run을 실행한다. 검증: agent가 slot 안에서 Milestone 파일을 작성/갱신하고 `develop`에 commit/push한다. +- [x] [cycle-develop-sync] sync layer가 `branches/develop` 또는 remote `develop`에서 pushed Milestone을 감지하고 원래 Plane 티켓 identity와 매칭한다. 검증: slot local 변경이나 push 실패 상태만으로 Plane `Todo` projection을 수행하지 않는다. +- [x] [cycle-plane-todo] 원본 Plane 본문을 `사용자 요청:` 댓글로 보존하고, `develop`의 Milestone 내용으로 Plane 본문/제목을 갱신한 뒤 `Todo`로 이동한다. 검증: 사용자에게 Plane Todo에서 `develop` 반영 결과를 검토할 수 있는 상태가 된다. - [ ] [cycle-idempotency] 같은 Plane 티켓 또는 같은 Milestone path 재처리 시 중복 생성 없이 남은 단계만 재시도한다. 검증: identity map, actor guard, provider revision, roadmap revision 기준으로 부분 실패를 복구한다. ### Epic: [project-config] Project sync configuration @@ -146,9 +146,9 @@ Plane-origin 생성 경로를 `develop` agent-roadmap 기준의 `Todo` 검토 - Checkout 기준: 안정성을 우선해 branch workspace와 각 slot은 같은 git remote를 기준으로 독립 checkout을 갖는 작업공간으로 둔다. 초기 workspace 생성 때 clone 비용이 들지만 이후 branch sync와 agent 작업은 각 checkout 안에서 분리되므로 운영 리스크를 낮춘다. - 후속 UI/UX: project settings 화면에서 Plane project target, git remote, workspace 설정을 확인하고 수정하는 UX가 필요하다. - 완료 근거(2026-06-07): `[project-config]`는 `services/core/internal/projectsync/`, `services/core/migrations/00004_create_project_sync_settings.sql`, `services/core/migrations/00005_create_workspace_slots.sql`, `services/core/queries/project_sync_settings.sql`, `services/core/queries/workspace_slots.sql`, `packages/contracts/notes/flutter-core-api-candidates.md`의 모델/저장/slot/UI 후보 경계와 테스트로 확인했다. 검증: `cd services/core && go test ./...` PASS. -- 현황 동기화(2026-06-10): 활성 `agent-task` 산출물은 없고, git worktree는 clean이다. 코드 기준 완료 근거는 `[project-config]`와 `workitempipeline`의 project binding/slot metadata 연결까지 확인했으며, branch/slot workspace topology 갱신, IOP Edge HTTP authoring run, pushed Milestone identity map, Plane `Todo` projection은 아직 완료 evidence가 없어 미완료로 유지한다. -- 중지 지점: project sync 설정과 기존 workspace slot/checkout metadata는 준비되었고, 다음 실행 단위는 `workspace-topology`의 branch/slot path 갱신과 `agent-bridge`의 `iop-http-channel` 정의/구현이다. Plane-origin 생성이 Plane `Todo` 상위 티켓과 `develop` agent-roadmap Milestone identity로 연결된 상태까지는 아직 도달 근거가 없다. -- 루프 목표: Plane `Backlog + AGENT assignee` -> `slots/` workspace 예약/준비 -> IOP Edge HTTP로 workspace agent authoring run 실행 -> agent가 Milestone 파일 작성/갱신 후 `develop` push -> sync layer가 `branches/develop` 또는 remote `develop`에서 push 완료/roadmap scan을 감지 -> Plane 원문 댓글 보존/본문 치환/제목 변경/`Todo` 이동 -> 사용자가 확인 후 `In Progress`로 이동하는 지점까지다. 현재는 `[project-config]` 완료 이후 `workspace-topology`와 `iop-http-channel`부터 미완료다. `In Progress` 이후 실행은 후속 잠금 범위다. +- 현황 동기화(2026-06-13): `agent-task/archive/2026/06/m-milestone-work-item-creation-sync/*/complete.log`의 `Roadmap Completion` 근거로 `cycle-workspace-topology`, `cycle-iop-http-ready`, `cycle-authoring-run`, `cycle-develop-sync`, `cycle-plane-todo`를 완료 처리했다. 현재 active 작업은 `agent-task/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry`(`cycle-idempotency`)이며, 이 Task는 완료 evidence가 없어 미완료로 유지한다. +- 중지 지점: project sync 설정, workspace topology, IOP Edge HTTP authoring run 경계, pushed Milestone identity match, Plane 원문 댓글 보존/본문·제목 갱신/`Todo` 이동 projection은 완료 근거가 있다. 다음 실행 단위는 identity map 기반 idempotency/retry다. +- 루프 목표: Plane `Backlog + AGENT assignee` -> `slots/` workspace 예약/준비 -> IOP Edge HTTP로 workspace agent authoring run 실행 -> agent가 Milestone 파일 작성/갱신 후 `develop` push -> sync layer가 `branches/develop` 또는 remote `develop`에서 push 완료/roadmap scan을 감지 -> Plane 원문 댓글 보존/본문 치환/제목 변경/`Todo` 이동 -> 사용자가 확인 후 `In Progress`로 이동하는 지점까지다. 현재 최상위 사이클에서 남은 실행 단위는 `cycle-idempotency`이며, `In Progress` 이후 실행은 후속 잠금 범위다. - 후속 잠금: `Todo -> In Progress` 이후 실행 lifecycle은 사용자가 명시적으로 잠금을 해제할 때까지 진행하지 않는다. - Plane work item 경계: 이 마일스톤에서 말하는 Plane Milestone은 Plane native milestone object가 아니라 1상위 티켓 = 1 agent-roadmap Milestone으로 보는 work item mapping이다. - 확인 필요: 없음 diff --git a/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/code_review_cloud_G07_0.log b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/code_review_cloud_G07_0.log new file mode 100644 index 0000000..9329959 --- /dev/null +++ b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/code_review_cloud_G07_0.log @@ -0,0 +1,141 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> Fill implementation-owned sections, run verification, keep active files in place, and report ready for review. Do not prompt the user directly during implementation. + +## 개요 + +date=2026-06-13 +task=m-milestone-work-item-creation-sync/04+03_plane_todo_projection, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` +- Task ids: + - `cycle-plane-todo`: 원본 Plane 본문을 `사용자 요청:` 댓글로 보존하고, `develop`의 Milestone 내용으로 Plane 본문/제목을 갱신한 뒤 `Todo`로 이동한다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 종결 절차는 코드리뷰 에이전트 전용이다. + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] Plane Body And Title Patch | [x] | +| [API-2] Projection Service Ordering | [x] | +| [API-3] Plane Body Formatter | [x] | + +## 구현 체크리스트 + +- [x] Plane adapter에 work item body/title update capability를 additive로 추가한다. +- [x] Projection service가 댓글 보존 -> body/title patch -> Todo status 이동 순서로 실행하고 중간 실패 시 다음 단계를 건너뛰게 한다. +- [x] Milestone 내용을 Plane body로 변환하는 작은 formatter를 추가하고 제목은 `[milestone-id] 제목`으로 만든다. +- [x] develop projection candidate가 없으면 no-op 또는 explicit not-ready error를 반환한다. +- [x] `cd services/core && go test ./...`와 `git diff --check`를 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] active 파일을 `.log`로 아카이브하고 PASS이면 `complete.log` 작성 후 task directory를 archive로 이동한다. +- [x] PASS이고 task group이 `m-`이면 런타임 완료 이벤트 메타데이터를 보고한다. + +## 계획 대비 변경 사항 + +- 의존성 상태 차이: 계획 본문은 predecessor `03_develop_match`의 `complete.log`가 missing이라 명시하지만, 해당 산출물은 이후 `roadmapsync` 패키지로 커밋(`dde07f9`)되어 코드 의존성(`ProjectionCandidate`, `Identity`, `Revision`, `MatchDevelopMilestone`)이 이미 존재한다. 따라서 코드 차원 선행 의존성은 충족된 상태로 보고 구현을 진행했다. `03` task 디렉터리는 active tree에 없고 archive 접근은 하지 않았다. +- API-1 provider.go 반영 방식: adapter에 `UpdateWorkItem`만 추가하는 대신, provider-neutral 경계 유지를 위해 `workitem`에 `CapabilityBodyProjection`, `BodyProjector` 인터페이스, `BodyProjection` 타입을 추가하고 Plane `Client.ProjectBody`로 구현했다. 이는 status projection과 body projection을 독립 단계로 유지하라는 도메인 규칙(상태/본문 분리)에 맞춘 선택이다. +- 기존 `UpdateIssueStatus`는 제거 없이 `UpdateWorkItem` wrapper로 재구성해 호출부 호환을 유지했다(계획의 "wrapper로 유지" 허용 범위). + +## 주요 설계 결정 + +- PATCH는 non-empty field만 담는다(`UpdateWorkItem`이 `name`/`description_html`/`state`를 개별 판단). blank title/body가 기존 값을 지우지 않으며, 모든 field가 비면 `ErrInvalidInput`으로 no-op patch를 막는다. +- `ProjectPlaneTodo`는 provider DTO를 직접 받지 않고 `PlaneTodoProvider`(Commenter+BodyProjector+StatusProjector) facet과 neutral 입력만 받는다. 순서는 comment(`사용자 요청:` 보존) → body/title → Todo status이며, 각 단계 실패 시 다음 단계를 호출하지 않는다. +- not-ready 후보는 provider를 한 번도 호출하지 않고 `ErrNotReady`를 반환한다. 필수 입력(ref/original body/todo state/milestone markdown) 누락은 `ErrInvalidProjectionInput`으로 조기 차단한다. +- formatter는 Milestone 제목 + `목표`/`범위` 섹션만 HTML-escape하여 본문에 싣는다. Provider identity 블록(provider, work item id 등 secret-like metadata)은 본문 섹션이 아니므로 투영에서 제외된다. 제목은 `[] 제목` 형식이며 slug는 Milestone 파일명에서 도출한다. +- 실제 Plane dev server Todo 이동 smoke는 plan 범위 밖이므로 adapter httptest와 projection fake로 보존 순서/patch body/status를 검증했다. raw token은 코드/테스트/문서에 기록하지 않았다. + +## 사용자 리뷰 요청 + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- comment 보존 실패가 body/status mutation을 막는지 확인한다. +- Todo 이동이 develop projection candidate 없이 실행되지 않는지 확인한다. +- Plane custom field 전제를 넣지 않았는지 확인한다. + +## 검증 결과 + +### API-1 중간 검증 +```bash +$ cd services/core && go test ./internal/adapters/plane ./internal/workitem +ok github.com/nomadcode/nomadcode-core/internal/adapters/plane 0.007s +ok github.com/nomadcode/nomadcode-core/internal/workitem 0.003s +``` + +### API-2 / API-3 중간 검증 +```bash +$ cd services/core && go test ./internal/roadmapsync ./internal/adapters/plane +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.003s +ok github.com/nomadcode/nomadcode-core/internal/adapters/plane (cached) +``` + +### 최종 검증 +```bash +$ cd services/core && go test ./... +ok github.com/nomadcode/nomadcode-core/cmd/plane-smoke (cached) +? github.com/nomadcode/nomadcode-core/cmd/server [no test files] +ok github.com/nomadcode/nomadcode-core/internal/adapters/a2a (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/jira 0.008s +ok github.com/nomadcode/nomadcode-core/internal/adapters/mattermost (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/openai (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/plane (cached) +ok github.com/nomadcode/nomadcode-core/internal/authoring (cached) +ok github.com/nomadcode/nomadcode-core/internal/config (cached) +ok github.com/nomadcode/nomadcode-core/internal/http 0.004s +ok github.com/nomadcode/nomadcode-core/internal/notification (cached) +ok github.com/nomadcode/nomadcode-core/internal/projectsync (cached) +ok github.com/nomadcode/nomadcode-core/internal/protosocket 0.010s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync (cached) +ok github.com/nomadcode/nomadcode-core/internal/scheduler (cached) +ok github.com/nomadcode/nomadcode-core/internal/storage (cached) +ok github.com/nomadcode/nomadcode-core/internal/workflow (cached) +ok github.com/nomadcode/nomadcode-core/internal/workitem (cached) +ok github.com/nomadcode/nomadcode-core/internal/workitempipeline 0.003s + +$ git diff --check +OK: no whitespace errors +``` + +--- + +## 코드리뷰 결과 + +- 종합 판정: PASS +- 차원별 평가: + - correctness: Pass + - completeness: Pass + - test coverage: Pass + - API contract: Pass + - code quality: Pass + - plan deviation: Pass + - verification trust: Pass +- 발견된 문제: 없음 +- 리뷰 중 조치: `services/core/internal/roadmapsync/plane_format_test.go`의 formatting-only drift를 `gofmt`로 정리했다. 동작/API 변경은 없다. +- 검증: + - `cd services/core && go test ./internal/adapters/plane ./internal/workitem` - PASS + - `cd services/core && go test ./internal/roadmapsync ./internal/adapters/plane` - PASS + - `cd services/core && go test ./...` - PASS + - `cd services/core && gofmt -l ./internal/roadmapsync ./internal/adapters/plane ./internal/workitem` - PASS; output 없음 + - `git diff --check` - PASS; output 없음 +- 다음 단계: PASS 절차로 active plan/review를 `.log`로 아카이브하고 `complete.log` 작성 후 task directory를 archive로 이동한다. diff --git a/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/complete.log b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/complete.log new file mode 100644 index 0000000..20a35e4 --- /dev/null +++ b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/complete.log @@ -0,0 +1,45 @@ +# Complete - m-milestone-work-item-creation-sync/04+03_plane_todo_projection + +## 완료 일시 + +2026-06-13 + +## 요약 + +Plane Todo projection slice를 1회 루프로 검토 완료했다. 최종 판정은 PASS다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_cloud_G07_0.log` | `code_review_cloud_G07_0.log` | PASS | Plane 원문 댓글 보존, develop Milestone 본문/제목 projection, Todo 상태 이동 순서와 실패 중단 조건을 확인했다. | + +## 구현/정리 내용 + +- Plane adapter에 body/title projection capability를 additive로 추가하고 기존 status update wrapper 호환을 유지했다. +- `roadmapsync.ProjectPlaneTodo`가 comment preserve -> body/title patch -> Todo status 순서로 실행되며 중간 실패 시 후속 mutation을 멈추도록 구현했다. +- develop Milestone Markdown을 Plane title/body로 변환하는 formatter와 관련 단위 테스트를 추가했다. +- 리뷰 중 `services/core/internal/roadmapsync/plane_format_test.go`의 formatting-only drift를 `gofmt`로 정리했다. + +## 최종 검증 + +- `cd services/core && go test ./internal/adapters/plane ./internal/workitem` - PASS; Plane adapter/workitem capability tests passed. +- `cd services/core && go test ./internal/roadmapsync ./internal/adapters/plane` - PASS; projection ordering/formatter and Plane adapter tests passed. +- `cd services/core && go test ./...` - PASS; all core packages passed. +- `cd services/core && gofmt -l ./internal/roadmapsync ./internal/adapters/plane ./internal/workitem` - PASS; output 없음. +- `git diff --check` - PASS; output 없음. + +## Roadmap Completion + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` +- Completed task ids: + - `cycle-plane-todo`: PASS; evidence=`agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/plan_cloud_G07_0.log`, `agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/code_review_cloud_G07_0.log`; verification=`cd services/core && go test ./...`, `git diff --check` +- Not completed task ids: 없음 + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/PLAN-cloud-G07.md b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/plan_cloud_G07_0.log similarity index 100% rename from agent-task/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/PLAN-cloud-G07.md rename to agent-task/archive/2026/06/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/plan_cloud_G07_0.log diff --git a/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_0.log b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_0.log new file mode 100644 index 0000000..892ffb2 --- /dev/null +++ b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_0.log @@ -0,0 +1,152 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> Fill implementation-owned sections, run verification, keep active files in place, and report ready for review. Do not prompt the user directly during implementation. + +## 개요 + +date=2026-06-13 +task=m-milestone-work-item-creation-sync/05+03,04_idempotency_retry, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` +- Task ids: + - `cycle-idempotency`: 같은 Plane 티켓 또는 같은 Milestone path 재처리 시 중복 생성 없이 남은 단계만 재시도한다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 종결 절차는 코드리뷰 에이전트 전용이다. + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] Identity Map Persistence | [x] | +| [API-2] Step Ledger | [x] | +| [API-3] Retry Decision Service | [x] | + +## 구현 체크리스트 + +- [x] Milestone path, provider, tenant, project, work item id, provider revision, roadmap revision을 저장하는 identity map migration/query/store를 추가한다. +- [x] provider work item 또는 Milestone path 재처리 시 existing identity를 찾고 중복 Milestone 생성/authoring을 막는다. +- [x] develop match, comment preserve, body/title update, Todo status move의 step ledger를 저장하고 성공한 step은 재시도하지 않는다. +- [x] actor guard와 revision mismatch가 silent overwrite 대신 conflict/retryable result를 반환하게 한다. +- [x] `cd services/core && go test ./...`와 `git diff --check`를 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] active 파일을 `.log`로 아카이브하고 FAIL follow-up `PLAN-cloud-G07.md` / `CODE_REVIEW-cloud-G07.md`를 작성한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [ ] PASS이고 task group이 `m-`이면 런타임 완료 이벤트 메타데이터를 보고한다. + +## 계획 대비 변경 사항 + +- 계획의 dependency 메모는 `03_develop_match`/`04_plane_todo_projection` complete.log가 missing이라고 적혀 있으나, 현재 두 predecessor 모두 `agent-task/archive/2026/06/m-milestone-work-item-creation-sync/03+02_develop_match/complete.log`와 `04+03_plane_todo_projection/complete.log`로 아카이브되어 있어 선행 의존성이 충족된 상태로 진행했다. (CLAUDE.md의 split subtask 선행 의존성 확인 예외에 따라 동일 task group의 후보 complete.log 존재만 확인) +- 계획은 identity 충돌(`conflict sentinel`)을 query 단계에서 처리하는 것처럼 표현했으나, `(provider, tenant, project, work_item_id)`와 `(roadmap_milestone_path)` 두 unique index는 단일 `ON CONFLICT` target으로 묶을 수 없다. 따라서 store 계층 `UpsertRoadmapSyncIdentity`에서 두 lookup을 선행 비교해 mismatched pair를 `ErrRoadmapSyncIdentityConflict`로 반환하도록 했다. raw unique-index 위반 대신 typed conflict로 막는다. +- API-3의 retry decision service는 계획대로 `internal/roadmapsync`에 두되, 기존 패키지가 `workitem`만 import하는 경계를 유지하기 위해 `db`/`storage`를 import하지 않는다. 호출자가 persisted identity(`ExistingIdentity`)와 completed step set(`map[Step]bool`)을 주입하는 provider-neutral 시그니처(`ReconcileCreationCycle`)로 설계했다. +- 함수명은 계획의 `ReconcileCreationCycle` 그대로 사용했다. + +## 주요 설계 결정 + +- **이중 unique index로 양방향 중복 차단**: 같은 Plane 티켓(`provider+tenant+project+work_item_id`)과 같은 Milestone path(`roadmap_milestone_path`) 양쪽에 unique index를 두어, 어느 쪽으로 재처리해도 동일 identity row를 재사용한다. upsert는 work item key 기준 `ON CONFLICT ... DO UPDATE`이며, path가 다른 ticket에 이미 묶여 있거나 ticket이 다른 path에 묶여 있으면 conflict로 막는다. +- **step ledger의 idempotent upsert**: `(roadmap_sync_identity_id, step)` unique index + `ON CONFLICT DO UPDATE`(no-op self-assign)로 이미 완료한 step을 재마킹해도 원본 `completed_at`을 유지하는 no-op이 된다. step은 DB CHECK과 store `Valid()` 양쪽에서 closed set(`develop_matched`, `original_comment_preserved`, `plane_body_updated`, `plane_todo_moved`)으로 제한하고, unknown step은 DB를 건드리기 전에 `ErrUnknownRoadmapSyncStep`으로 거부한다. +- **resume 결정은 ordered step의 first-incomplete**: `ReconcileCreationCycle`은 completed set에 없는 첫 ordered step부터 재개(`ActionResume`)하고, 전부 완료면 `ActionComplete`를 반환한다. 성공한 step은 다시 호출되지 않는다. +- **actor guard**: trigger actor가 `SelfActor`(NomadCode self mutation)와 같으면 `ActionSkipSelfMutation`을 반환해 NomadCode가 직접 만든 body/title 변경이 새 authoring trigger로 오인되지 않게 한다. +- **revision mismatch는 silent overwrite 대신 conflict**: 이미 identity가 존재할 때 trigger의 provider/roadmap revision이 persisted 값과 다르면 `ActionConflict`를 반환한다. 단, 둘 중 하나가 빈 값(미기록)이면 첫 cycle을 막지 않도록 conflict로 보지 않는다. +- **generated db code 수동 작성**: 로컬에 sqlc 바이너리가 없어(`bin/migrate-up`은 goose만 `go run`으로 사용) 기존 `workspace_slots.sql.go` 등과 동일한 sqlc v1.31.1 스타일로 `*.sql.go`와 `models.go` 구조체를 손으로 작성했다. nullable `provider_revision`/`roadmap_revision`은 `sqlc.yaml`의 `emit_pointers_for_null_types`에 맞춰 `*string`으로 두었다. + +## 사용자 리뷰 요청 + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- provider work item id와 Milestone path 양쪽 unique path가 중복 생성을 막는지 확인한다. +- completed step이 재시도에서 다시 호출되지 않는지 확인한다. +- actor guard가 NomadCode self mutation을 새 user trigger로 처리하지 않는지 확인한다. + +## 검증 결과 + +### API-1 중간 검증 +```bash +$ cd services/core && go test ./internal/storage ./internal/db +ok github.com/nomadcode/nomadcode-core/internal/storage 0.003s +? github.com/nomadcode/nomadcode-core/internal/db [no test files] +``` + +### API-2 중간 검증 +```bash +$ cd services/core && go test ./internal/storage +ok github.com/nomadcode/nomadcode-core/internal/storage 0.004s +``` + +### API-3 중간 검증 +```bash +$ cd services/core && go test ./internal/roadmapsync ./internal/storage +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.004s +ok github.com/nomadcode/nomadcode-core/internal/storage (cached) +``` + +### 최종 검증 +```bash +$ cd services/core && go test ./... +ok github.com/nomadcode/nomadcode-core/cmd/plane-smoke (cached) +? github.com/nomadcode/nomadcode-core/cmd/server [no test files] +ok github.com/nomadcode/nomadcode-core/internal/adapters/a2a (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/jira (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/mattermost (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/openai (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/plane (cached) +? github.com/nomadcode/nomadcode-core/internal/agent [no test files] +ok github.com/nomadcode/nomadcode-core/internal/authoring 0.009s +ok github.com/nomadcode/nomadcode-core/internal/config (cached) +? github.com/nomadcode/nomadcode-core/internal/db [no test files] +ok github.com/nomadcode/nomadcode-core/internal/http 0.004s +? github.com/nomadcode/nomadcode-core/internal/model [no test files] +ok github.com/nomadcode/nomadcode-core/internal/notification (cached) +ok github.com/nomadcode/nomadcode-core/internal/projectsync (cached) +ok github.com/nomadcode/nomadcode-core/internal/protosocket 0.012s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync (cached) +ok github.com/nomadcode/nomadcode-core/internal/scheduler 2.013s +ok github.com/nomadcode/nomadcode-core/internal/storage (cached) +ok github.com/nomadcode/nomadcode-core/internal/workflow 0.004s +ok github.com/nomadcode/nomadcode-core/internal/workitem (cached) +ok github.com/nomadcode/nomadcode-core/internal/workitempipeline (cached) + +$ git diff --check +(출력 없음, exit 0) + +$ cd services/core && go vet ./internal/roadmapsync ./internal/storage ./internal/db && gofmt -l internal/roadmapsync internal/storage internal/db +(출력 없음, vet exit 0 / gofmt 차이 없음) +``` + +--- + +## 코드리뷰 결과 + +- 종합 판정: FAIL +- 차원별 평가: + - correctness: Fail + - completeness: Fail + - test coverage: Fail + - API contract: Fail + - code quality: Pass + - plan deviation: Fail + - verification trust: Warn +- 발견된 문제: + - Required: `services/core/queries/roadmap_sync_identities.sql:16`의 `ON CONFLICT (provider, tenant, project, work_item_id) DO UPDATE`가 기존 row의 `roadmap_milestone_path` 일치 조건 없이 path를 덮어쓴다. `services/core/internal/storage/roadmap_sync_identities.go:55`의 선조회는 순차 실행에서는 충돌을 잡지만 원자적이지 않다. 두 재처리가 같은 provider work item에 서로 다른 Milestone path로 동시에 들어오면 둘 다 선조회에서 no rows를 본 뒤, 나중 upsert가 기존 identity를 새 path로 조용히 재바인딩할 수 있다. 수정: upsert SQL에 `WHERE roadmap_sync_identities.roadmap_milestone_path = EXCLUDED.roadmap_milestone_path` 같은 조건부 update를 추가하고, 조건 불일치로 `RETURNING` row가 없거나 path unique 위반이 발생하면 `ErrRoadmapSyncIdentityConflict`로 변환한다. 이 경로를 단위 테스트로 고정한다. + - Required: `services/core/internal/roadmapsync/identity.go:100`의 `Identity.Matches`가 `tenant`와 `project`를 비교하지 않아, `services/core/internal/roadmapsync/retry.go:117`의 retry decision이 같은 provider/work item id/path라도 다른 Plane workspace/project identity를 같은 identity로 재사용할 수 있다. 이번 plan과 DB unique key는 provider, tenant, project, work item id를 identity key로 저장하므로 retry decision도 동일한 키를 비교해야 한다. 수정: retry/develop matching에서 쓰는 identity match가 tenant/project까지 비교하도록 보완하거나, retry 전용 strict identity match를 도입하고 tenant/project mismatch 테스트를 추가한다. + - Suggested: `./bin/sqlc` 재생성을 시도했지만 `queries/workspace_slots.sql:19:11: column reference "project_sync_setting_id" is ambiguous`로 실패해 신규 generated DB 파일이 실제 sqlc 산출물과 일치하는지 도구로 재검증하지 못했다. follow-up에서는 위 Required 수정 뒤 `go test`와 함께 이 생성 경로의 현재 차단 사유를 명시하거나, 가능한 경우 sqlc 재생성 증거를 남긴다. +- 다음 단계: FAIL follow-up plan/review를 생성해 Required 이슈를 수정하고 검증한다. diff --git a/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_1.log b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_1.log new file mode 100644 index 0000000..690e652 --- /dev/null +++ b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_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`. +> 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. + +## 개요 + +date=2026-06-13 +task=m-milestone-work-item-creation-sync/05+03,04_idempotency_retry, plan=1, tag=REVIEW_API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` +- Task ids: + - `cycle-idempotency`: 같은 Plane 티켓 또는 같은 Milestone path 재처리 시 중복 생성 없이 남은 단계만 재시도한다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-{review_lane}-GNN.md` -> `code_review_{review_lane}_GNN_N.log`, `PLAN-{build_lane}-GNN.md` -> `plan_{build_lane}_GNN_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task directory를 archive로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 수정이나 `update-roadmap` 직접 호출은 하지 않는다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_API-1] Atomic Identity Conflict Handling | [x] | +| [REVIEW_API-2] Strict Retry Identity Key | [x] | + +## 구현 체크리스트 + +- [x] `UpsertRoadmapSyncIdentityByWorkItem`가 기존 provider work item row의 Milestone path가 다르면 update하지 않고 typed conflict로 반환하게 SQL/query/store/tests를 보완한다. +- [x] path unique race/raw unique violation을 `ErrRoadmapSyncIdentityConflict`로 변환하고 테스트를 추가한다. +- [x] retry identity comparison이 provider, tenant, project, work item id, Milestone path를 같은 identity key로 비교하도록 보완하고 mismatch 테스트를 추가한다. +- [x] `./bin/sqlc` 재생성 또는 차단 증거, `go test -count=1 ./internal/storage ./internal/roadmapsync ./internal/adapters/plane`, `go test ./...`, `git diff --check` 검증 결과를 남긴다. +- [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/{task_name}/`를 `agent-task/archive/YYYY/MM/{task_name}/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [x] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [x] PASS split 작업이면 이동 후 빈 active parent `agent-task/{task_group}/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-{build_lane}-GNN.md`와 `CODE_REVIEW-{review_lane}-GNN.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 사용자 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +- **REVIEW_API-1**: 계획은 `DO UPDATE`에 path 일치 조건을 두고 no-row를 conflict로 변환하라고 했고 그대로 구현했다. 추가로, conflict 보호를 단일 statement 내에서 원자적으로 만들기 위해 `DO UPDATE`의 SET 목록에서 `roadmap_milestone_path = EXCLUDED.roadmap_milestone_path`를 제거했다(기존 path를 유지). 그리고 첫 리뷰에서 비원자적이라 지적된 store 계층의 선조회(by-path/by-work-item lookup) 2건을 upsert 전에서 제거했다 — 이제 보호는 전적으로 SQL(conditional `DO UPDATE` + path unique index)이 담당하므로 read-then-write race가 사라진다. `GetRoadmapSyncIdentityByWorkItem`/`ByMilestonePath` lookup 함수 자체는 다른 호출자용으로 유지했다. +- **REVIEW_API-2**: 계획이 제시한 두 옵션 중 "retry 전용 strict matcher 분리" 쪽을 택했다. 기존 `Identity.Matches`는 develop matching(`MatchDevelopMilestone`)이 사용하고 `TestIdentityMatches`가 "비-key 필드(project 등)는 무시한다"를 명시적으로 보장하므로, `Matches`를 바꾸면 선행 PASS subtask의 develop-match 동작이 바뀐다. 그래서 `Identity.MatchesStrict`(provider, tenant, project, work item id, path 전부 비교)를 새로 추가하고 `ReconcileCreationCycle`에서만 사용했다. develop match/parser 동작은 변경하지 않았으므로 해당 기존 테스트도 그대로 통과한다. +- **REVIEW_API-2 conflict 처리**: strict match 실패 시 단순히 `IdentityKept=false`로 두고 fresh resume하면 tenant/project mismatch가 조용히 새 cycle로 처리된다. 리뷰 의도(다른 workspace/project를 같은 identity로 재사용 금지)에 맞춰, `Existing.Found`이지만 strict match가 아니면 `ActionConflict`(`reasonIdentityKeyMismatch`)를 반환하도록 했다. +- **`./bin/sqlc` 검증 명령**: 계획대로 실행했으나, 계획·첫 리뷰가 예고한 대로 기존 `queries/workspace_slots.sql:19`의 `project_sync_setting_id is ambiguous`로 실패했다(내 신규 query 2개와 무관, sqlc가 첫 package 에러에서 중단). exact 출력을 아래 검증 결과에 남기고, generated DB 코드는 기존 sqlc v1.31.1 스타일로 수동 갱신했다. 이 ambiguity 수정은 이번 follow-up 범위(Required 이슈)에 포함되지 않아 건드리지 않았다. + +## 주요 설계 결정 + +- **원자적 conflict 보호 (read-then-write 제거)**: `UpsertRoadmapSyncIdentityByWorkItem`은 work item key로 `ON CONFLICT ... DO UPDATE`하되, `WHERE roadmap_sync_identities.roadmap_milestone_path = EXCLUDED.roadmap_milestone_path`로 기존 path와 incoming path가 같을 때만 update한다. SET 목록에서 path를 제거해 기존 path는 절대 덮어쓰지 않는다. + - 같은 work item + 다른 path 동시 재처리 → `DO UPDATE`가 매칭 row 없음 → `RETURNING` empty → `pgx.ErrNoRows` → store가 `ErrRoadmapSyncIdentityConflict`로 매핑. 두 동시 재처리 중 누구도 기존 identity를 새 path로 재바인딩할 수 없다. + - 다른 work item + 기존 path 재사용 → path unique index(SQLSTATE `23505`) 위반 → `pgconn.PgError.Code == "23505"`를 `ErrRoadmapSyncIdentityConflict`로 매핑(raw pg error 노출 안 함). + - 그 외 DB 에러는 그대로 전파(conflict로 삼키지 않음). 단위 테스트로 4개 경로(성공+conditional shape, no-row conflict, unique-violation conflict, generic error 전파)를 고정했다. 실제 multi-process DB race e2e는 환경상 제외하고 query shape/error mapping unit test로 대체했다. +- **strict retry identity key**: retry decision은 `Identity.MatchesStrict`로 provider+tenant+project+work item id+path를 모두 비교한다. 이는 DB의 provider-side unique key(`provider, tenant, project, work_item_id`)와 정확히 일치하므로, project-scoped sync 계약을 위반하는 cross-workspace/project identity 재사용을 막는다. loose `Matches`는 develop matching 전용으로 남겨 기존 동작을 보존했다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 사용자 결정, 사용자 소유 외부 환경/secret/서비스 준비, 또는 계획 범위 변경 없이는 안전하게 진행할 수 없으면 아래 항목을 실제 내용으로 교체하고, 구현을 중단한 뒤 active 파일을 그대로 둔 채 리뷰를 요청한다. 구현 에이전트는 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 후속 에이전트가 명령 재실행이나 산출물 수집으로 해소할 수 있는 검증 증거 공백만으로는 사용자 리뷰 요청을 작성하지 않는다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- identity upsert가 provider work item conflict에서 다른 Milestone path로 재바인딩하지 않는지 확인한다. +- path unique constraint 위반과 conditional update no-row가 typed conflict로 변환되는지 확인한다. +- retry identity comparison이 tenant/project mismatch를 같은 identity로 취급하지 않는지 확인한다. +- `./bin/sqlc` 재생성 성공 또는 차단 증거가 검증 결과에 정확히 기록되었는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. + +### REVIEW_API-1 중간 검증 +```bash +$ cd services/core && go test -count=1 ./internal/storage +ok github.com/nomadcode/nomadcode-core/internal/storage 0.003s +``` + +### REVIEW_API-2 중간 검증 +```bash +$ cd services/core && go test -count=1 ./internal/roadmapsync +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.003s +``` + +### 최종 검증 +```bash +$ cd services/core && ./bin/sqlc +# package db +queries/workspace_slots.sql:19:11: column reference "project_sync_setting_id" is ambiguous +exit status 1 +# (차단됨) 기존 workspace_slots query의 ambiguity로 실패. 신규 +# roadmap_sync_identities.sql / roadmap_sync_steps.sql 는 에러를 내지 않았고 +# sqlc 가 첫 package 에러에서 중단함. 이번 follow-up Required 범위 밖이라 +# 미수정. generated DB 코드는 기존 sqlc v1.31.1 스타일로 수동 갱신했다. + +$ cd services/core && go test -count=1 ./internal/storage ./internal/roadmapsync ./internal/adapters/plane +ok github.com/nomadcode/nomadcode-core/internal/storage 0.004s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.003s +ok github.com/nomadcode/nomadcode-core/internal/adapters/plane 0.008s + +$ cd services/core && go test ./... +ok github.com/nomadcode/nomadcode-core/cmd/plane-smoke 0.005s +? github.com/nomadcode/nomadcode-core/cmd/server [no test files] +ok github.com/nomadcode/nomadcode-core/internal/adapters/a2a (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/jira 0.009s +ok github.com/nomadcode/nomadcode-core/internal/adapters/mattermost (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/openai (cached) +ok github.com/nomadcode/nomadcode-core/internal/adapters/plane 0.013s +? github.com/nomadcode/nomadcode-core/internal/agent [no test files] +ok github.com/nomadcode/nomadcode-core/internal/authoring 0.004s +ok github.com/nomadcode/nomadcode-core/internal/config (cached) +? github.com/nomadcode/nomadcode-core/internal/db [no test files] +ok github.com/nomadcode/nomadcode-core/internal/http 0.004s +? github.com/nomadcode/nomadcode-core/internal/model [no test files] +ok github.com/nomadcode/nomadcode-core/internal/notification (cached) +ok github.com/nomadcode/nomadcode-core/internal/projectsync (cached) +ok github.com/nomadcode/nomadcode-core/internal/protosocket 0.019s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.004s +ok github.com/nomadcode/nomadcode-core/internal/scheduler (cached) +ok github.com/nomadcode/nomadcode-core/internal/storage 0.008s +ok github.com/nomadcode/nomadcode-core/internal/workflow 0.007s +ok github.com/nomadcode/nomadcode-core/internal/workitem 0.005s +ok github.com/nomadcode/nomadcode-core/internal/workitempipeline 0.007s + +$ git diff --check +(출력 없음, exit 0) + +$ cd services/core && go vet ./internal/roadmapsync ./internal/storage ./internal/db && gofmt -l internal/roadmapsync internal/storage internal/db +(출력 없음, vet exit 0 / gofmt 차이 없음) +``` + +--- + +> **[IMPLEMENTING AGENT - BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 코드리뷰 결과 + +- 종합 판정: PASS +- 차원별 평가: + - correctness: Pass + - completeness: Pass + - test coverage: Pass + - API contract: Pass + - code quality: Pass + - plan deviation: Pass + - verification trust: Pass +- 발견된 문제: 없음 +- 다음 단계: PASS이므로 `complete.log` 작성 후 task directory를 archive로 이동한다. diff --git a/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/complete.log b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/complete.log new file mode 100644 index 0000000..793d31b --- /dev/null +++ b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/complete.log @@ -0,0 +1,47 @@ +# Complete - m-milestone-work-item-creation-sync/05+03,04_idempotency_retry + +## 완료 일시 + +2026-06-13 + +## 요약 + +Identity map idempotency/retry follow-up loop 2에서 atomic identity conflict handling과 strict retry identity key를 보완하고 최종 PASS로 종결했다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_cloud_G07_0.log` | `code_review_cloud_G07_0.log` | FAIL | identity upsert rebind race와 retry identity tenant/project mismatch 문제로 follow-up 작성 | +| `plan_cloud_G07_1.log` | `code_review_cloud_G07_1.log` | PASS | conditional upsert conflict mapping과 strict retry identity tests 확인 | + +## 구현/정리 내용 + +- `UpsertRoadmapSyncIdentityByWorkItem`의 `DO UPDATE`를 기존 Milestone path와 incoming path가 같을 때만 실행하도록 바꾸고, path 재바인딩 no-row와 unique violation을 `ErrRoadmapSyncIdentityConflict`로 변환했다. +- store의 비원자적 선조회 기반 conflict check를 제거하고 SQL statement 단위 conflict handling으로 정리했다. +- `Identity.MatchesStrict`를 추가하고 `ReconcileCreationCycle`에서 provider, tenant, project, work item id, Milestone path를 모두 비교하도록 보완했다. +- rebind/no-row, path unique violation, generic DB error propagation, tenant/project mismatch 테스트를 추가했다. + +## 최종 검증 + +- `cd services/core && ./bin/sqlc` - BLOCKED; 기존 `queries/workspace_slots.sql:19:11` ambiguity로 실패, 이번 신규 query와 follow-up 범위 밖 차단으로 기록됨. +- `cd services/core && go test -count=1 ./internal/storage ./internal/roadmapsync ./internal/adapters/plane` - PASS; storage, roadmapsync, Plane adapter targeted tests 통과. +- `cd services/core && go test ./...` - PASS; core 전체 Go test 통과. +- `cd services/core && go vet ./internal/roadmapsync ./internal/storage ./internal/db` - PASS; 출력 없음. +- `cd services/core && gofmt -l internal/roadmapsync internal/storage internal/db` - PASS; 출력 없음. +- `git diff --check` - PASS; whitespace 오류 없음. + +## Roadmap Completion + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` +- Completed task ids: + - `cycle-idempotency`: PASS; evidence=`agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_1.log`, `agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/code_review_cloud_G07_1.log`; verification=`cd services/core && go test -count=1 ./internal/storage ./internal/roadmapsync ./internal/adapters/plane`, `cd services/core && go test ./...`, `git diff --check` +- Not completed task ids: 없음 + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/PLAN-cloud-G07.md b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_0.log similarity index 100% rename from agent-task/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/PLAN-cloud-G07.md rename to agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_0.log diff --git a/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_1.log b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_1.log new file mode 100644 index 0000000..d4ca0a6 --- /dev/null +++ b/agent-task/archive/2026/06/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/plan_cloud_G07_1.log @@ -0,0 +1,94 @@ + + +# Plan - Review API Idempotency Retry Fix + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-cloud-G07.md`의 구현 에이전트 소유 섹션을 반드시 채운다. 사용자 전용 결정이나 외부 환경 차단은 review stub의 `사용자 리뷰 요청`에 근거를 기록하고 멈춘다. 구현 중 사용자에게 직접 질문하거나 선택지를 제시하지 않는다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` +- Task ids: + - `cycle-idempotency`: 같은 Plane 티켓 또는 같은 Milestone path 재처리 시 중복 생성 없이 남은 단계만 재시도한다. +- Completion mode: check-on-pass + +## 배경 + +첫 리뷰(`code_review_cloud_G07_0.log`)는 identity map/retry slice를 FAIL로 판정했다. 선조회 후 upsert가 동시 재처리에서 같은 provider work item을 다른 Milestone path로 조용히 재바인딩할 수 있고, retry identity comparison이 `tenant`/`project`를 identity key로 비교하지 않아 project-scoped sync 계약과 어긋난다. + +## 범위 결정 근거 + +- 이번 follow-up은 `code_review_cloud_G07_0.log`의 Required 이슈만 닫는다. +- Plane Todo projection, formatter, adapter body projection은 선행 subtask PASS 범위이므로 동작 변경하지 않는다. +- `./bin/sqlc`는 현재 `queries/workspace_slots.sql`의 기존 ambiguity로 실패할 수 있다. 재생성이 성공하면 산출물을 반영하고, 실패하면 정확한 출력과 함께 generated DB 코드가 수동 갱신되었음을 검증 결과에 남긴다. + +## 구현 체크리스트 + +- [ ] `UpsertRoadmapSyncIdentityByWorkItem`가 기존 provider work item row의 Milestone path가 다르면 update하지 않고 typed conflict로 반환하게 SQL/query/store/tests를 보완한다. +- [ ] path unique race/raw unique violation을 `ErrRoadmapSyncIdentityConflict`로 변환하고 테스트를 추가한다. +- [ ] retry identity comparison이 provider, tenant, project, work item id, Milestone path를 같은 identity key로 비교하도록 보완하고 mismatch 테스트를 추가한다. +- [ ] `./bin/sqlc` 재생성 또는 차단 증거, `go test -count=1 ./internal/storage ./internal/roadmapsync ./internal/adapters/plane`, `go test ./...`, `git diff --check` 검증 결과를 남긴다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## [REVIEW_API-1] Atomic Identity Conflict Handling + +### 문제 + +`services/core/queries/roadmap_sync_identities.sql`의 upsert는 provider work item conflict 시 `roadmap_milestone_path`를 무조건 `EXCLUDED` 값으로 갱신한다. `services/core/internal/storage/roadmap_sync_identities.go`의 선조회는 원자적이지 않아, 같은 work item에 서로 다른 path가 동시에 들어오면 나중 upsert가 기존 identity를 재바인딩할 수 있다. + +### 해결 방법 + +- upsert SQL의 `DO UPDATE`에 기존 `roadmap_milestone_path`가 incoming path와 같을 때만 update하는 조건을 둔다. +- 조건 불일치로 `RETURNING` row가 없을 때 `ErrRoadmapSyncIdentityConflict`로 변환한다. +- path unique constraint 위반도 raw pg error 대신 `ErrRoadmapSyncIdentityConflict`로 변환한다. +- mock 기반 단위 테스트로 query shape, no-row conflict mapping, unique violation mapping을 고정한다. 실제 DB race e2e가 없으면 생략 사유를 검증 결과에 남긴다. + +### 수정 파일 후보 + +- `services/core/queries/roadmap_sync_identities.sql` +- `services/core/internal/db/roadmap_sync_identities.sql.go` +- `services/core/internal/storage/roadmap_sync_identities.go` +- `services/core/internal/storage/roadmap_sync_identities_test.go` + +### 중간 검증 + +```bash +cd services/core && go test -count=1 ./internal/storage +``` + +## [REVIEW_API-2] Strict Retry Identity Key + +### 문제 + +`roadmapsync.Identity.Matches`는 provider, work item id, Milestone path만 비교한다. 이번 plan과 DB unique key는 provider, tenant, project, work item id를 provider-side identity key로 저장하므로 retry decision도 다른 Plane workspace/project를 같은 identity로 재사용하면 안 된다. + +### 해결 방법 + +- retry decision에서 쓰는 identity comparison이 `provider`, `tenant`, `project`, `work_item_id`, `roadmap_milestone_path`를 모두 비교하도록 보완한다. +- 기존 broader matching이 다른 call site에 필요하면 retry 전용 strict matcher를 분리하고 `ReconcileCreationCycle`에서만 strict matcher를 사용한다. +- tenant/project mismatch가 `ActionConflict`가 되는 테스트를 추가한다. develop match 동작을 함께 바꾸는 경우 기존 parser/match 테스트도 업데이트한다. + +### 수정 파일 후보 + +- `services/core/internal/roadmapsync/identity.go` +- `services/core/internal/roadmapsync/retry.go` +- `services/core/internal/roadmapsync/identity_test.go` +- `services/core/internal/roadmapsync/retry_test.go` + +### 중간 검증 + +```bash +cd services/core && go test -count=1 ./internal/roadmapsync +``` + +## 최종 검증 + +```bash +cd services/core && ./bin/sqlc +cd services/core && go test -count=1 ./internal/storage ./internal/roadmapsync ./internal/adapters/plane +cd services/core && go test ./... +git diff --check +``` + +`./bin/sqlc`가 기존 `workspace_slots` query ambiguity로 실패하면, exact stdout/stderr를 `CODE_REVIEW-cloud-G07.md`에 남기고 신규 generated 파일의 수동 변경 근거를 함께 기록한다. diff --git a/agent-task/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/CODE_REVIEW-cloud-G07.md b/agent-task/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/CODE_REVIEW-cloud-G07.md deleted file mode 100644 index 502279f..0000000 --- a/agent-task/m-milestone-work-item-creation-sync/04+03_plane_todo_projection/CODE_REVIEW-cloud-G07.md +++ /dev/null @@ -1,92 +0,0 @@ - - -# Code Review Reference - API - -> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** -> Fill implementation-owned sections, run verification, keep active files in place, and report ready for review. Do not prompt the user directly during implementation. - -## 개요 - -date=2026-06-13 -task=m-milestone-work-item-creation-sync/04+03_plane_todo_projection, plan=0, tag=API - -## Roadmap Targets - -- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` -- Task ids: - - `cycle-plane-todo`: 원본 Plane 본문을 `사용자 요청:` 댓글로 보존하고, `develop`의 Milestone 내용으로 Plane 본문/제목을 갱신한 뒤 `Todo`로 이동한다. -- Completion mode: check-on-pass - -## 이 파일을 읽는 리뷰 에이전트에게 - -> **[REVIEW AGENT ONLY]** 종결 절차는 코드리뷰 에이전트 전용이다. - -## 구현 항목별 완료 여부 - -| 항목 | 완료 여부 | -|------|---------| -| [API-1] Plane Body And Title Patch | [ ] | -| [API-2] Projection Service Ordering | [ ] | -| [API-3] Plane Body Formatter | [ ] | - -## 구현 체크리스트 - -- [ ] Plane adapter에 work item body/title update capability를 additive로 추가한다. -- [ ] Projection service가 댓글 보존 -> body/title patch -> Todo status 이동 순서로 실행하고 중간 실패 시 다음 단계를 건너뛰게 한다. -- [ ] Milestone 내용을 Plane body로 변환하는 작은 formatter를 추가하고 제목은 `[milestone-id] 제목`으로 만든다. -- [ ] develop projection candidate가 없으면 no-op 또는 explicit not-ready error를 반환한다. -- [ ] `cd services/core && go test ./...`와 `git diff --check`를 실행한다. -- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. - -## 코드리뷰 전용 체크리스트 - -- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. -- [ ] active 파일을 `.log`로 아카이브하고 PASS이면 `complete.log` 작성 후 task directory를 archive로 이동한다. -- [ ] PASS이고 task group이 `m-`이면 런타임 완료 이벤트 메타데이터를 보고한다. - -## 계획 대비 변경 사항 - -_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ - -## 주요 설계 결정 - -_구현 에이전트가 주요 설계 결정 사항을 기록한다._ - -## 사용자 리뷰 요청 - -- 상태: 없음 -- 사유 유형: 없음 -- 결정 필요: 없음 -- 차단 근거: 없음 -- 실행한 검증/명령: 없음 -- 자동 후속 불가 이유: 없음 -- 재개 조건: 없음 - -## 리뷰어를 위한 체크포인트 - -- comment 보존 실패가 body/status mutation을 막는지 확인한다. -- Todo 이동이 develop projection candidate 없이 실행되지 않는지 확인한다. -- Plane custom field 전제를 넣지 않았는지 확인한다. - -## 검증 결과 - -### API-1 중간 검증 -```bash -$ cd services/core && go test ./internal/adapters/plane ./internal/workitem -(output) -``` - -### API-2 중간 검증 -```bash -$ cd services/core && go test ./internal/roadmapsync ./internal/adapters/plane -(output) -``` - -### 최종 검증 -```bash -$ cd services/core && go test ./... -$ git diff --check -(output) -``` - ---- diff --git a/agent-task/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/CODE_REVIEW-cloud-G07.md b/agent-task/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/CODE_REVIEW-cloud-G07.md deleted file mode 100644 index c0b5499..0000000 --- a/agent-task/m-milestone-work-item-creation-sync/05+03,04_idempotency_retry/CODE_REVIEW-cloud-G07.md +++ /dev/null @@ -1,92 +0,0 @@ - - -# Code Review Reference - API - -> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** -> Fill implementation-owned sections, run verification, keep active files in place, and report ready for review. Do not prompt the user directly during implementation. - -## 개요 - -date=2026-06-13 -task=m-milestone-work-item-creation-sync/05+03,04_idempotency_retry, plan=0, tag=API - -## Roadmap Targets - -- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md` -- Task ids: - - `cycle-idempotency`: 같은 Plane 티켓 또는 같은 Milestone path 재처리 시 중복 생성 없이 남은 단계만 재시도한다. -- Completion mode: check-on-pass - -## 이 파일을 읽는 리뷰 에이전트에게 - -> **[REVIEW AGENT ONLY]** 종결 절차는 코드리뷰 에이전트 전용이다. - -## 구현 항목별 완료 여부 - -| 항목 | 완료 여부 | -|------|---------| -| [API-1] Identity Map Persistence | [ ] | -| [API-2] Step Ledger | [ ] | -| [API-3] Retry Decision Service | [ ] | - -## 구현 체크리스트 - -- [ ] Milestone path, provider, tenant, project, work item id, provider revision, roadmap revision을 저장하는 identity map migration/query/store를 추가한다. -- [ ] provider work item 또는 Milestone path 재처리 시 existing identity를 찾고 중복 Milestone 생성/authoring을 막는다. -- [ ] develop match, comment preserve, body/title update, Todo status move의 step ledger를 저장하고 성공한 step은 재시도하지 않는다. -- [ ] actor guard와 revision mismatch가 silent overwrite 대신 conflict/retryable result를 반환하게 한다. -- [ ] `cd services/core && go test ./...`와 `git diff --check`를 실행한다. -- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. - -## 코드리뷰 전용 체크리스트 - -- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. -- [ ] active 파일을 `.log`로 아카이브하고 PASS이면 `complete.log` 작성 후 task directory를 archive로 이동한다. -- [ ] PASS이고 task group이 `m-`이면 런타임 완료 이벤트 메타데이터를 보고한다. - -## 계획 대비 변경 사항 - -_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ - -## 주요 설계 결정 - -_구현 에이전트가 주요 설계 결정 사항을 기록한다._ - -## 사용자 리뷰 요청 - -- 상태: 없음 -- 사유 유형: 없음 -- 결정 필요: 없음 -- 차단 근거: 없음 -- 실행한 검증/명령: 없음 -- 자동 후속 불가 이유: 없음 -- 재개 조건: 없음 - -## 리뷰어를 위한 체크포인트 - -- provider work item id와 Milestone path 양쪽 unique path가 중복 생성을 막는지 확인한다. -- completed step이 재시도에서 다시 호출되지 않는지 확인한다. -- actor guard가 NomadCode self mutation을 새 user trigger로 처리하지 않는지 확인한다. - -## 검증 결과 - -### API-1 중간 검증 -```bash -$ cd services/core && go test ./internal/storage ./internal/db -(output) -``` - -### API-3 중간 검증 -```bash -$ cd services/core && go test ./internal/roadmapsync ./internal/storage -(output) -``` - -### 최종 검증 -```bash -$ cd services/core && go test ./... -$ git diff --check -(output) -``` - ---- diff --git a/services/core/internal/adapters/openai/client.go b/services/core/internal/adapters/openai/client.go index 33c49a3..1d7bce4 100644 --- a/services/core/internal/adapters/openai/client.go +++ b/services/core/internal/adapters/openai/client.go @@ -260,7 +260,9 @@ func buildRequestMetadata(input model.GenerateInput) map[string]any { merged[k] = v } if input.WorkspaceMetadata != nil { - merged["workspace"] = input.WorkspaceMetadata + if path := strings.TrimSpace(input.WorkspaceMetadata.Path); path != "" { + merged["workspace"] = path + } } return merged } diff --git a/services/core/internal/adapters/openai/client_test.go b/services/core/internal/adapters/openai/client_test.go index ad560d3..ea56574 100644 --- a/services/core/internal/adapters/openai/client_test.go +++ b/services/core/internal/adapters/openai/client_test.go @@ -95,7 +95,7 @@ func TestGenerateCallsResponsesAPI(t *testing.T) { } } -func TestGenerateWorkspaceMetadataIsObject(t *testing.T) { +func TestGenerateWorkspaceMetadataUsesFlatWorkspacePath(t *testing.T) { var gotBody map[string]any server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -132,21 +132,12 @@ func TestGenerateWorkspaceMetadataIsObject(t *testing.T) { if metadata["task_id"] != "task-abc" || metadata["source"] != "plane" { t.Fatalf("flat metadata not preserved: %#v", metadata) } - ws, ok := metadata["workspace"].(map[string]any) + wsPath, ok := metadata["workspace"].(string) if !ok { - t.Fatalf("expected metadata.workspace object, got %T: %#v", metadata["workspace"], metadata["workspace"]) + t.Fatalf("expected metadata.workspace string, got %T: %#v", metadata["workspace"], metadata["workspace"]) } - if ws["path"] != "/home/user/workspace/nomadcode/slots/000" { - t.Fatalf("unexpected workspace.path: %#v", ws["path"]) - } - if ws["source_branch"] != "develop" { - t.Fatalf("unexpected workspace.source_branch: %#v", ws["source_branch"]) - } - if ws["provider"] != "plane" { - t.Fatalf("unexpected workspace.provider: %#v", ws["provider"]) - } - if ws["work_item_id"] != "NOMAD-42" { - t.Fatalf("unexpected workspace.work_item_id: %#v", ws["work_item_id"]) + if wsPath != "/home/user/workspace/nomadcode/slots/000" { + t.Fatalf("unexpected workspace path: %#v", wsPath) } } diff --git a/services/core/internal/adapters/plane/client.go b/services/core/internal/adapters/plane/client.go index 6234fea..7410e77 100644 --- a/services/core/internal/adapters/plane/client.go +++ b/services/core/internal/adapters/plane/client.go @@ -21,6 +21,7 @@ var ( _ workitem.Commenter = (*Client)(nil) _ workitem.StatusProjector = (*Client)(nil) _ workitem.Creator = (*Client)(nil) + _ workitem.BodyProjector = (*Client)(nil) ) var ( @@ -75,6 +76,17 @@ type UpdateIssueStatusInput struct { Status string } +// UpdateWorkItemInput carries an additive PATCH of a Plane work item. Only +// non-empty fields are sent, so a blank Title/DescriptionHTML/Status never +// clears an existing value. Status maps to the Plane `state` field, the same +// field UpdateIssueStatus patches. +type UpdateWorkItemInput struct { + Ref WorkItemRef + Title string + DescriptionHTML string + Status string +} + func NewClient(cfg Config, logger *slog.Logger) *Client { return &Client{cfg: cfg, http: http.DefaultClient, logger: logger} } @@ -96,6 +108,7 @@ func (c *Client) Capabilities() workitem.Capabilities { workitem.CapabilityComment: true, workitem.CapabilityStatusProjection: true, workitem.CapabilityCreate: true, + workitem.CapabilityBodyProjection: true, workitem.CapabilityLabelProjection: false, } } @@ -145,6 +158,21 @@ func (c *Client) ProjectStatus(ctx context.Context, input workitem.StatusProject }) } +func (c *Client) ProjectBody(ctx context.Context, input workitem.BodyProjection) error { + planeRef, err := planeWorkItemRef(input.Ref) + if err != nil { + return err + } + if strings.TrimSpace(input.Title) == "" && strings.TrimSpace(input.DescriptionHTML) == "" { + return ErrInvalidInput + } + return c.UpdateWorkItem(ctx, UpdateWorkItemInput{ + Ref: planeRef, + Title: input.Title, + DescriptionHTML: input.DescriptionHTML, + }) +} + func (c *Client) CreateWorkItem(ctx context.Context, input workitem.CreateInput) (workitem.CreateResult, error) { workspace := strings.TrimSpace(input.Tenant) project := strings.TrimSpace(input.Project) @@ -240,8 +268,29 @@ func (c *Client) AddComment(ctx context.Context, input AddCommentInput) error { } func (c *Client) UpdateIssueStatus(ctx context.Context, input UpdateIssueStatusInput) error { - body := map[string]string{ - "state": input.Status, + return c.UpdateWorkItem(ctx, UpdateWorkItemInput{ + Ref: input.Ref, + Status: input.Status, + }) +} + +// UpdateWorkItem PATCHes the work item with only the non-empty fields in input. +// name, description_html, and state are sent independently so a body/title +// update never disturbs the state, and a status move never disturbs the body. +// An input with no non-empty field is an invalid no-op patch. +func (c *Client) UpdateWorkItem(ctx context.Context, input UpdateWorkItemInput) error { + body := map[string]string{} + if title := strings.TrimSpace(input.Title); title != "" { + body["name"] = title + } + if descHTML := strings.TrimSpace(input.DescriptionHTML); descHTML != "" { + body["description_html"] = descHTML + } + if status := strings.TrimSpace(input.Status); status != "" { + body["state"] = status + } + if len(body) == 0 { + return ErrInvalidInput } return c.do(ctx, http.MethodPatch, workItemPath(input.Ref)+"/", body, nil) } diff --git a/services/core/internal/adapters/plane/client_test.go b/services/core/internal/adapters/plane/client_test.go index 6a2d307..685e933 100644 --- a/services/core/internal/adapters/plane/client_test.go +++ b/services/core/internal/adapters/plane/client_test.go @@ -150,11 +150,107 @@ func TestClientImplementsWorkItemFacets(t *testing.T) { if !caps.Supports(workitem.CapabilityCreate) { t.Error("expected create capability") } + if !caps.Supports(workitem.CapabilityBodyProjection) { + t.Error("expected body projection capability") + } if caps.Supports(workitem.CapabilityLabelProjection) { t.Error("label projection should be unsupported") } } +func TestUpdateWorkItemPatchesOnlyNonEmptyFields(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPatch { + t.Fatalf("method: got %s", r.Method) + } + if r.URL.Path != "/api/v1/workspaces/general/projects/project-1/work-items/work-1/" { + t.Fatalf("path: got %s", r.URL.Path) + } + var body map[string]string + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatalf("decode body: %v", err) + } + if body["name"] != "New title" { + t.Errorf("name: got %q", body["name"]) + } + if body["description_html"] != "

new body

" { + t.Errorf("description_html: got %q", body["description_html"]) + } + if _, ok := body["state"]; ok { + t.Errorf("state should not be present when blank: %#v", body) + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client := NewClient(Config{BaseURL: server.URL, Token: "test-token"}, nil) + err := client.UpdateWorkItem(context.Background(), UpdateWorkItemInput{ + Ref: WorkItemRef{ + WorkspaceSlug: "general", + ProjectID: "project-1", + WorkItemID: "work-1", + }, + Title: "New title", + DescriptionHTML: "

new body

", + }) + if err != nil { + t.Fatalf("UpdateWorkItem returned error: %v", err) + } +} + +func TestUpdateWorkItemRejectsEmptyPatch(t *testing.T) { + client := NewClient(Config{BaseURL: "https://example.com", Token: "test-token"}, nil) + err := client.UpdateWorkItem(context.Background(), UpdateWorkItemInput{ + Ref: WorkItemRef{WorkspaceSlug: "general", ProjectID: "project-1", WorkItemID: "work-1"}, + }) + if err == nil || !strings.Contains(err.Error(), ErrInvalidInput.Error()) { + t.Fatalf("empty patch: expected ErrInvalidInput, got %v", err) + } +} + +func TestProjectBodyPatchesTitleAndBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPatch { + t.Fatalf("method: got %s", r.Method) + } + if r.URL.Path != "/api/v1/workspaces/general/projects/project-1/work-items/work-1/" { + t.Fatalf("path: got %s", r.URL.Path) + } + var body map[string]string + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatalf("decode body: %v", err) + } + if body["name"] != "[m-slug] Title" { + t.Errorf("name: got %q", body["name"]) + } + if body["description_html"] != "

milestone

" { + t.Errorf("description_html: got %q", body["description_html"]) + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client := NewClient(Config{BaseURL: server.URL, Token: "test-token"}, nil) + err := client.ProjectBody(context.Background(), workitem.BodyProjection{ + Ref: workitem.Ref{Provider: "plane", Tenant: "general", Project: "project-1", ID: "work-1"}, + Title: "[m-slug] Title", + DescriptionHTML: "

milestone

", + }) + if err != nil { + t.Fatalf("ProjectBody returned error: %v", err) + } +} + +func TestProjectBodyRejectsEmptyInput(t *testing.T) { + client := NewClient(Config{BaseURL: "https://example.com", Token: "test-token"}, nil) + err := client.ProjectBody(context.Background(), workitem.BodyProjection{ + Ref: workitem.Ref{Provider: "plane", Tenant: "general", Project: "project-1", ID: "work-1"}, + }) + if err == nil || !strings.Contains(err.Error(), ErrInvalidInput.Error()) { + t.Fatalf("empty body projection: expected ErrInvalidInput, got %v", err) + } +} + func TestCreateIssuePostsWorkItem(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { diff --git a/services/core/internal/db/models.go b/services/core/internal/db/models.go index 36d4187..6f7be07 100644 --- a/services/core/internal/db/models.go +++ b/services/core/internal/db/models.go @@ -24,6 +24,29 @@ type ProjectSyncSetting struct { UpdatedAt time.Time `json:"updated_at"` } +type RoadmapSyncIdentity struct { + ID int64 `json:"id"` + Shape string `json:"shape"` + RoadmapMilestonePath string `json:"roadmap_milestone_path"` + RoadmapItemID string `json:"roadmap_item_id"` + Provider string `json:"provider"` + Tenant string `json:"tenant"` + Project string `json:"project"` + WorkItemID string `json:"work_item_id"` + ParentWorkItemID string `json:"parent_work_item_id"` + ProviderRevision *string `json:"provider_revision"` + RoadmapRevision *string `json:"roadmap_revision"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +type RoadmapSyncStep struct { + ID int64 `json:"id"` + RoadmapSyncIdentityID int64 `json:"roadmap_sync_identity_id"` + Step string `json:"step"` + CompletedAt time.Time `json:"completed_at"` +} + type Task struct { ID string `json:"id"` Title string `json:"title"` diff --git a/services/core/internal/db/roadmap_sync_identities.sql.go b/services/core/internal/db/roadmap_sync_identities.sql.go new file mode 100644 index 0000000..4d47f86 --- /dev/null +++ b/services/core/internal/db/roadmap_sync_identities.sql.go @@ -0,0 +1,138 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: roadmap_sync_identities.sql + +package db + +import ( + "context" +) + +const getRoadmapSyncIdentityByMilestonePath = `-- name: GetRoadmapSyncIdentityByMilestonePath :one +SELECT id, shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision, created_at, updated_at +FROM roadmap_sync_identities +WHERE roadmap_milestone_path = $1 +` + +func (q *Queries) GetRoadmapSyncIdentityByMilestonePath(ctx context.Context, roadmapMilestonePath string) (RoadmapSyncIdentity, error) { + row := q.db.QueryRow(ctx, getRoadmapSyncIdentityByMilestonePath, roadmapMilestonePath) + var i RoadmapSyncIdentity + err := row.Scan( + &i.ID, + &i.Shape, + &i.RoadmapMilestonePath, + &i.RoadmapItemID, + &i.Provider, + &i.Tenant, + &i.Project, + &i.WorkItemID, + &i.ParentWorkItemID, + &i.ProviderRevision, + &i.RoadmapRevision, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const getRoadmapSyncIdentityByWorkItem = `-- name: GetRoadmapSyncIdentityByWorkItem :one +SELECT id, shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision, created_at, updated_at +FROM roadmap_sync_identities +WHERE provider = $1 AND tenant = $2 AND project = $3 AND work_item_id = $4 +` + +type GetRoadmapSyncIdentityByWorkItemParams struct { + Provider string `json:"provider"` + Tenant string `json:"tenant"` + Project string `json:"project"` + WorkItemID string `json:"work_item_id"` +} + +func (q *Queries) GetRoadmapSyncIdentityByWorkItem(ctx context.Context, arg GetRoadmapSyncIdentityByWorkItemParams) (RoadmapSyncIdentity, error) { + row := q.db.QueryRow(ctx, getRoadmapSyncIdentityByWorkItem, + arg.Provider, + arg.Tenant, + arg.Project, + arg.WorkItemID, + ) + var i RoadmapSyncIdentity + err := row.Scan( + &i.ID, + &i.Shape, + &i.RoadmapMilestonePath, + &i.RoadmapItemID, + &i.Provider, + &i.Tenant, + &i.Project, + &i.WorkItemID, + &i.ParentWorkItemID, + &i.ProviderRevision, + &i.RoadmapRevision, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const upsertRoadmapSyncIdentityByWorkItem = `-- name: UpsertRoadmapSyncIdentityByWorkItem :one +INSERT INTO roadmap_sync_identities ( + shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision +) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) +ON CONFLICT (provider, tenant, project, work_item_id) +DO UPDATE SET + shape = EXCLUDED.shape, + roadmap_item_id = EXCLUDED.roadmap_item_id, + parent_work_item_id = EXCLUDED.parent_work_item_id, + provider_revision = EXCLUDED.provider_revision, + roadmap_revision = EXCLUDED.roadmap_revision, + updated_at = now() +WHERE roadmap_sync_identities.roadmap_milestone_path = EXCLUDED.roadmap_milestone_path +RETURNING id, shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision, created_at, updated_at +` + +type UpsertRoadmapSyncIdentityByWorkItemParams struct { + Shape string `json:"shape"` + RoadmapMilestonePath string `json:"roadmap_milestone_path"` + RoadmapItemID string `json:"roadmap_item_id"` + Provider string `json:"provider"` + Tenant string `json:"tenant"` + Project string `json:"project"` + WorkItemID string `json:"work_item_id"` + ParentWorkItemID string `json:"parent_work_item_id"` + ProviderRevision *string `json:"provider_revision"` + RoadmapRevision *string `json:"roadmap_revision"` +} + +func (q *Queries) UpsertRoadmapSyncIdentityByWorkItem(ctx context.Context, arg UpsertRoadmapSyncIdentityByWorkItemParams) (RoadmapSyncIdentity, error) { + row := q.db.QueryRow(ctx, upsertRoadmapSyncIdentityByWorkItem, + arg.Shape, + arg.RoadmapMilestonePath, + arg.RoadmapItemID, + arg.Provider, + arg.Tenant, + arg.Project, + arg.WorkItemID, + arg.ParentWorkItemID, + arg.ProviderRevision, + arg.RoadmapRevision, + ) + var i RoadmapSyncIdentity + err := row.Scan( + &i.ID, + &i.Shape, + &i.RoadmapMilestonePath, + &i.RoadmapItemID, + &i.Provider, + &i.Tenant, + &i.Project, + &i.WorkItemID, + &i.ParentWorkItemID, + &i.ProviderRevision, + &i.RoadmapRevision, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} diff --git a/services/core/internal/db/roadmap_sync_steps.sql.go b/services/core/internal/db/roadmap_sync_steps.sql.go new file mode 100644 index 0000000..9fdf0d3 --- /dev/null +++ b/services/core/internal/db/roadmap_sync_steps.sql.go @@ -0,0 +1,67 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: roadmap_sync_steps.sql + +package db + +import ( + "context" +) + +const listRoadmapSyncSteps = `-- name: ListRoadmapSyncSteps :many +SELECT id, roadmap_sync_identity_id, step, completed_at +FROM roadmap_sync_steps +WHERE roadmap_sync_identity_id = $1 +ORDER BY id +` + +func (q *Queries) ListRoadmapSyncSteps(ctx context.Context, roadmapSyncIdentityID int64) ([]RoadmapSyncStep, error) { + rows, err := q.db.Query(ctx, listRoadmapSyncSteps, roadmapSyncIdentityID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []RoadmapSyncStep + for rows.Next() { + var i RoadmapSyncStep + if err := rows.Scan( + &i.ID, + &i.RoadmapSyncIdentityID, + &i.Step, + &i.CompletedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const markRoadmapSyncStepCompleted = `-- name: MarkRoadmapSyncStepCompleted :one +INSERT INTO roadmap_sync_steps (roadmap_sync_identity_id, step) +VALUES ($1, $2) +ON CONFLICT (roadmap_sync_identity_id, step) +DO UPDATE SET roadmap_sync_identity_id = EXCLUDED.roadmap_sync_identity_id +RETURNING id, roadmap_sync_identity_id, step, completed_at +` + +type MarkRoadmapSyncStepCompletedParams struct { + RoadmapSyncIdentityID int64 `json:"roadmap_sync_identity_id"` + Step string `json:"step"` +} + +func (q *Queries) MarkRoadmapSyncStepCompleted(ctx context.Context, arg MarkRoadmapSyncStepCompletedParams) (RoadmapSyncStep, error) { + row := q.db.QueryRow(ctx, markRoadmapSyncStepCompleted, arg.RoadmapSyncIdentityID, arg.Step) + var i RoadmapSyncStep + err := row.Scan( + &i.ID, + &i.RoadmapSyncIdentityID, + &i.Step, + &i.CompletedAt, + ) + return i, err +} diff --git a/services/core/internal/model/model.go b/services/core/internal/model/model.go index 879e500..4406cb4 100644 --- a/services/core/internal/model/model.go +++ b/services/core/internal/model/model.go @@ -9,10 +9,10 @@ type Client interface { Generate(ctx context.Context, input GenerateInput) (GenerateResult, error) } -// WorkspaceMetadata carries workspace-bound authoring context passed as -// metadata.workspace in the /v1/responses payload. Fields here map directly -// to IOP Edge workspace routing fields so the receiving agent can resolve the -// correct slot checkout without CLI-level invocation by NomadCode. +// WorkspaceMetadata carries workspace-bound authoring context. On the wire, +// only the Path is serialized as a flat string to metadata.workspace in the +// /v1/responses payload. Other fields are used on the client-side/authoring +// logic. type WorkspaceMetadata struct { Path string `json:"path"` SourceBranch string `json:"source_branch,omitempty"` diff --git a/services/core/internal/roadmapsync/identity.go b/services/core/internal/roadmapsync/identity.go index 693c57b..dbe6254 100644 --- a/services/core/internal/roadmapsync/identity.go +++ b/services/core/internal/roadmapsync/identity.go @@ -102,6 +102,30 @@ func (i Identity) Matches(other Identity) bool { left.RoadmapMilestonePath == right.RoadmapMilestonePath } +// MatchesStrict reports whether two identities refer to the same item using the +// full provider-side identity key persisted by the identity map: provider, +// tenant, project, work item id, plus the Milestone path. Unlike Matches it +// also compares tenant and project, so a re-process in a different Plane +// workspace/project is never treated as the same identity. The retry decision +// uses this so it cannot reuse an identity across project-scoped sync targets. +// Callers must be able to normalize both sides; an un-normalizable identity +// never matches. +func (i Identity) MatchesStrict(other Identity) bool { + left, err := i.Normalize() + if err != nil { + return false + } + right, err := other.Normalize() + if err != nil { + return false + } + return left.Provider == right.Provider && + left.Tenant == right.Tenant && + left.Project == right.Project && + left.WorkItemID == right.WorkItemID && + left.RoadmapMilestonePath == right.RoadmapMilestonePath +} + // Revision captures the branch and revision context that a scan observed, so a // projection decision can record what develop state it was based on. Milestone // sync defaults Branch to develop per the contract note. diff --git a/services/core/internal/roadmapsync/identity_test.go b/services/core/internal/roadmapsync/identity_test.go index 514ec6a..0f3ec12 100644 --- a/services/core/internal/roadmapsync/identity_test.go +++ b/services/core/internal/roadmapsync/identity_test.go @@ -88,6 +88,36 @@ func TestIdentityMatches(t *testing.T) { } } +func TestIdentityMatchesStrict(t *testing.T) { + base := Identity{ + RoadmapMilestonePath: "agent-roadmap/phase/p/milestones/m.md", + Provider: "plane", + Tenant: "general", + Project: "NOMAD", + WorkItemID: "NOMAD-13", + } + if !base.MatchesStrict(base) { + t.Errorf("identical identity must strict-match") + } + + differentTenant := base + differentTenant.Tenant = "other-workspace" + if base.MatchesStrict(differentTenant) { + t.Errorf("different tenant must not strict-match") + } + + differentProject := base + differentProject.Project = "OTHER" + if base.MatchesStrict(differentProject) { + t.Errorf("different project must not strict-match") + } + + // Matches (loose) ignores tenant/project; MatchesStrict must not. + if !base.Matches(differentTenant) { + t.Errorf("sanity: loose Matches should still ignore tenant") + } +} + func TestRevisionNormalizeDefaultsDevelop(t *testing.T) { got := Revision{}.Normalize() if got.Branch != DefaultBranch { diff --git a/services/core/internal/roadmapsync/plane_format.go b/services/core/internal/roadmapsync/plane_format.go new file mode 100644 index 0000000..a47f8d9 --- /dev/null +++ b/services/core/internal/roadmapsync/plane_format.go @@ -0,0 +1,148 @@ +package roadmapsync + +import ( + "bufio" + "fmt" + "html" + "path" + "strings" +) + +// PlaneProjection is the provider-neutral, HTML-safe view of a develop +// Milestone that should be written into the Plane work item. Title follows the +// `[milestone-slug] Milestone Title` form (Milestone doc line 103); BodyHTML is +// the Milestone goal/scope rendered as escaped HTML. No provider secret or +// token is ever placed here. +type PlaneProjection struct { + Title string + BodyHTML string +} + +// MilestoneSlug derives the milestone id used in the Plane title from the +// Milestone file path: the file name without its .md extension +// (agent-roadmap/phase//milestones/.md -> ). It returns "" +// when the path is empty or not a .md file so a caller never builds a +// `[] Title` prefix from a bad path. +func MilestoneSlug(milestonePath string) string { + cleaned := normalizeRepoPath(milestonePath) + if cleaned == "" { + return "" + } + base := path.Base(cleaned) + if !strings.HasSuffix(base, ".md") { + return "" + } + return strings.TrimSuffix(base, ".md") +} + +// FormatPlaneProjection turns a develop Milestone Markdown document into the +// Plane title/body projection. milestonePath supplies the slug used in the +// title prefix. The body carries the Milestone title, the "## 목표"/"## 범위" +// sections when present, and is HTML-escaped so untrusted Markdown text never +// injects markup into Plane. The formatter is deliberately minimal: it does not +// attempt full Markdown rendering, only a safe, readable summary. +func FormatPlaneProjection(milestonePath, markdown string) PlaneProjection { + slug := MilestoneSlug(milestonePath) + title := extractMilestoneTitle(markdown) + + displayTitle := title + if slug != "" { + shown := title + if strings.TrimSpace(shown) == "" { + shown = slug + } + displayTitle = fmt.Sprintf("[%s] %s", slug, shown) + } + + return PlaneProjection{ + Title: strings.TrimSpace(displayTitle), + BodyHTML: buildBodyHTML(title, markdown), + } +} + +// extractMilestoneTitle returns the text of the first level-1 heading +// ("# Title") in the document, or "" when there is none. +func extractMilestoneTitle(markdown string) string { + scanner := bufio.NewScanner(strings.NewReader(markdown)) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if strings.HasPrefix(line, "# ") { + return strings.TrimSpace(strings.TrimPrefix(line, "#")) + } + } + return "" +} + +// sectionLabels are the Milestone section headings whose body lines are carried +// into the Plane projection, in output order. Matched case-insensitively after +// stripping heading markers. +var sectionLabels = []string{"목표", "범위"} + +// buildBodyHTML renders the Milestone title plus the goal/scope sections as +// escaped HTML paragraphs. Lines under a tracked section are emitted until the +// next heading. Everything is HTML-escaped, so neither Markdown text nor any +// stray provider metadata is rendered as live markup. +func buildBodyHTML(title, markdown string) string { + var b strings.Builder + if t := strings.TrimSpace(title); t != "" { + b.WriteString("

") + b.WriteString(html.EscapeString(t)) + b.WriteString("

") + } + + sections := collectSections(markdown) + for _, label := range sectionLabels { + lines, ok := sections[label] + if !ok || len(lines) == 0 { + continue + } + b.WriteString("

") + b.WriteString(html.EscapeString(label)) + b.WriteString("

") + for _, line := range lines { + b.WriteString("

") + b.WriteString(html.EscapeString(line)) + b.WriteString("

") + } + } + return b.String() +} + +// collectSections groups non-empty body lines under each tracked section +// heading. A heading line that matches one of sectionLabels (after trimming +// `#`) opens that section; any heading closes the current one. Keys are the +// matched label. +func collectSections(markdown string) map[string][]string { + want := map[string]bool{} + for _, l := range sectionLabels { + want[l] = true + } + + sections := map[string][]string{} + current := "" + scanner := bufio.NewScanner(strings.NewReader(markdown)) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" { + continue + } + if strings.HasPrefix(line, "#") { + heading := strings.TrimSpace(strings.TrimLeft(line, "#")) + if want[heading] { + current = heading + } else { + current = "" + } + continue + } + if current == "" { + continue + } + text := strings.TrimSpace(strings.TrimLeft(line, "-*")) + if text == "" { + continue + } + sections[current] = append(sections[current], text) + } + return sections +} diff --git a/services/core/internal/roadmapsync/plane_format_test.go b/services/core/internal/roadmapsync/plane_format_test.go new file mode 100644 index 0000000..c1f5e53 --- /dev/null +++ b/services/core/internal/roadmapsync/plane_format_test.go @@ -0,0 +1,73 @@ +package roadmapsync + +import ( + "strings" + "testing" +) + +const sampleMilestone = `# Milestone Work Item Creation Sync + +## 목표 + +- Plane 티켓을 develop Milestone과 연결한다. + +## 범위 + +- Plane 본문/제목 갱신과 Todo 이동. + +## Provider identity + +- provider: plane +- work item id: NOMAD-13 +` + +func TestFormatPlaneProjectionTitleUsesSlugPrefix(t *testing.T) { + proj := FormatPlaneProjection( + "agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/milestone-work-item-creation-sync.md", + sampleMilestone, + ) + want := "[milestone-work-item-creation-sync] Milestone Work Item Creation Sync" + if proj.Title != want { + t.Fatalf("title: got %q want %q", proj.Title, want) + } +} + +func TestFormatPlaneProjectionBodyIncludesGoalAndIsEscaped(t *testing.T) { + proj := FormatPlaneProjection( + "agent-roadmap/phase/p/milestones/m.md", + "# T\n\n## 목표\n\n- 목표 raw 본문\n", + ) + if !strings.Contains(proj.BodyHTML, "목표") { + t.Errorf("body should include goal section: %q", proj.BodyHTML) + } + // Raw markup in the Milestone text must be escaped, not rendered live. + if strings.Contains(proj.BodyHTML, "raw") { + t.Errorf("body must escape untrusted markup: %q", proj.BodyHTML) + } + if !strings.Contains(proj.BodyHTML, "<b>raw</b>") { + t.Errorf("body should contain escaped markup: %q", proj.BodyHTML) + } +} + +func TestFormatPlaneProjectionExcludesProviderIdentityBlock(t *testing.T) { + proj := FormatPlaneProjection("agent-roadmap/phase/p/milestones/m.md", sampleMilestone) + // Provider/work item id metadata is not a body section, so it must not leak + // into the Plane body. + if strings.Contains(proj.BodyHTML, "NOMAD-13") || strings.Contains(proj.BodyHTML, "provider") { + t.Errorf("body must not include provider identity metadata: %q", proj.BodyHTML) + } +} + +func TestMilestoneSlug(t *testing.T) { + cases := map[string]string{ + "agent-roadmap/phase/p/milestones/m-foo.md": "m-foo", + "m-foo.md": "m-foo", + "": "", + "agent-roadmap/x.txt": "", + } + for in, want := range cases { + if got := MilestoneSlug(in); got != want { + t.Errorf("MilestoneSlug(%q): got %q want %q", in, got, want) + } + } +} diff --git a/services/core/internal/roadmapsync/plane_projection.go b/services/core/internal/roadmapsync/plane_projection.go new file mode 100644 index 0000000..142bc3d --- /dev/null +++ b/services/core/internal/roadmapsync/plane_projection.go @@ -0,0 +1,102 @@ +package roadmapsync + +import ( + "context" + "errors" + "strings" + + "github.com/nomadcode/nomadcode-core/internal/workitem" +) + +// ErrNotReady is returned when the develop projection candidate is not ready, +// so a caller never silently projects a Plane Todo for an unmatched or unpushed +// Milestone. +var ErrNotReady = errors.New("develop projection candidate is not ready") + +// ErrInvalidProjectionInput is returned when a required projection input +// (provider ref, original body, todo state, or milestone content) is missing. +var ErrInvalidProjectionInput = errors.New("invalid plane todo projection input") + +// preserveCommentPrefix opens the comment that preserves the original Plane +// body before the develop content replaces it. Kept as a stable prefix so the +// preserved request is recognizable in the Plane timeline. +const preserveCommentPrefix = "사용자 요청:" + +// PlaneTodoProvider is the minimal set of provider facets the Todo projection +// drives, in execution order: preserve the original body as a comment, replace +// title/body, then move the work item to the Todo state. It is satisfied by the +// Plane adapter Client and by test fakes. +type PlaneTodoProvider interface { + workitem.Commenter + workitem.BodyProjector + workitem.StatusProjector +} + +// ProjectPlaneTodoInput carries everything ProjectPlaneTodo needs without +// reaching into provider DTOs: the develop match decision, the original Plane +// body to preserve, the Todo state id to move to, and the develop Milestone +// markdown to project. Ref is the provider work item the projection targets. +type ProjectPlaneTodoInput struct { + Candidate ProjectionCandidate + Ref workitem.Ref + OriginalBody string + TodoStateID string + // MilestoneMarkdown is the develop Milestone content used to build the new + // Plane title/body via FormatPlaneProjection. + MilestoneMarkdown string + // ExternalSource/ExternalID tag the preservation comment so a re-run can be + // recognized as nomadcode-authored rather than a user request. + ExternalSource string + ExternalID string +} + +// ProjectPlaneTodo runs the Plane Todo projection in the order the Milestone +// requires (doc lines 57/102): preserve the original body as a `사용자 요청:` +// comment, then replace the title/body with the develop Milestone content, then +// move the work item to the Todo state. A failure at any step stops the +// sequence so a later step never runs on an unpreserved or unupdated ticket. +// +// When the develop projection candidate is not ready it returns ErrNotReady +// without calling the provider, so a Plane Todo is never produced for an +// unmatched or unpushed Milestone. +func ProjectPlaneTodo(ctx context.Context, p PlaneTodoProvider, in ProjectPlaneTodoInput) error { + if !in.Candidate.Ready { + return ErrNotReady + } + if p == nil || + strings.TrimSpace(string(in.Ref.Provider)) == "" || + strings.TrimSpace(in.Ref.ID) == "" || + strings.TrimSpace(in.OriginalBody) == "" || + strings.TrimSpace(in.TodoStateID) == "" || + strings.TrimSpace(in.MilestoneMarkdown) == "" { + return ErrInvalidProjectionInput + } + + projection := FormatPlaneProjection(in.Candidate.Identity.RoadmapMilestonePath, in.MilestoneMarkdown) + + // 1. Preserve the original Plane body as a comment before anything mutates it. + if err := p.AppendComment(ctx, workitem.CommentInput{ + Ref: in.Ref, + Body: preserveCommentPrefix + " " + in.OriginalBody, + Format: workitem.CommentFormatHTML, + ExternalSource: in.ExternalSource, + ExternalID: in.ExternalID, + }); err != nil { + return err + } + + // 2. Replace title/body with the develop Milestone projection. + if err := p.ProjectBody(ctx, workitem.BodyProjection{ + Ref: in.Ref, + Title: projection.Title, + DescriptionHTML: projection.BodyHTML, + }); err != nil { + return err + } + + // 3. Move the work item to the Todo state. + return p.ProjectStatus(ctx, workitem.StatusProjection{ + Ref: in.Ref, + ProviderState: in.TodoStateID, + }) +} diff --git a/services/core/internal/roadmapsync/plane_projection_test.go b/services/core/internal/roadmapsync/plane_projection_test.go new file mode 100644 index 0000000..df08c31 --- /dev/null +++ b/services/core/internal/roadmapsync/plane_projection_test.go @@ -0,0 +1,127 @@ +package roadmapsync + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/nomadcode/nomadcode-core/internal/workitem" +) + +type fakeProvider struct { + calls []string + commentBody string + + commentErr error + bodyErr error + statusErr error + + bodyProjection workitem.BodyProjection + statusState string +} + +func (f *fakeProvider) AppendComment(_ context.Context, in workitem.CommentInput) error { + f.calls = append(f.calls, "comment") + f.commentBody = in.Body + return f.commentErr +} + +func (f *fakeProvider) ProjectBody(_ context.Context, in workitem.BodyProjection) error { + f.calls = append(f.calls, "body") + f.bodyProjection = in + return f.bodyErr +} + +func (f *fakeProvider) ProjectStatus(_ context.Context, in workitem.StatusProjection) error { + f.calls = append(f.calls, "status") + f.statusState = in.ProviderState + return f.statusErr +} + +func readyInput() ProjectPlaneTodoInput { + return ProjectPlaneTodoInput{ + Candidate: ProjectionCandidate{ + Ready: true, + Identity: Identity{ + RoadmapMilestonePath: "agent-roadmap/phase/p/milestones/m-foo.md", + }, + }, + Ref: workitem.Ref{Provider: "plane", Tenant: "general", Project: "p", ID: "work-1"}, + OriginalBody: "

original request

", + TodoStateID: "todo-state", + MilestoneMarkdown: "# Foo Milestone\n\n## 목표\n\n- 목표\n", + ExternalSource: "nomadcode", + ExternalID: "task-1", + } +} + +func TestProjectPlaneTodoSuccessOrderAndPreservation(t *testing.T) { + f := &fakeProvider{} + if err := ProjectPlaneTodo(context.Background(), f, readyInput()); err != nil { + t.Fatalf("ProjectPlaneTodo returned error: %v", err) + } + if got := strings.Join(f.calls, ","); got != "comment,body,status" { + t.Fatalf("call order: got %q", got) + } + if !strings.HasPrefix(f.commentBody, preserveCommentPrefix) { + t.Errorf("comment must preserve original body with prefix: %q", f.commentBody) + } + if !strings.Contains(f.commentBody, "original request") { + t.Errorf("comment must contain original body: %q", f.commentBody) + } + if f.statusState != "todo-state" { + t.Errorf("status: got %q", f.statusState) + } + if !strings.HasPrefix(f.bodyProjection.Title, "[m-foo]") { + t.Errorf("body title should use slug prefix: %q", f.bodyProjection.Title) + } +} + +func TestProjectPlaneTodoStopsWhenCommentFails(t *testing.T) { + f := &fakeProvider{commentErr: errors.New("comment boom")} + err := ProjectPlaneTodo(context.Background(), f, readyInput()) + if err == nil || !strings.Contains(err.Error(), "comment boom") { + t.Fatalf("expected comment error, got %v", err) + } + if got := strings.Join(f.calls, ","); got != "comment" { + t.Fatalf("only comment should run: got %q", got) + } +} + +func TestProjectPlaneTodoStopsWhenBodyFails(t *testing.T) { + f := &fakeProvider{bodyErr: errors.New("body boom")} + err := ProjectPlaneTodo(context.Background(), f, readyInput()) + if err == nil || !strings.Contains(err.Error(), "body boom") { + t.Fatalf("expected body error, got %v", err) + } + if got := strings.Join(f.calls, ","); got != "comment,body" { + t.Fatalf("status must not run after body failure: got %q", got) + } +} + +func TestProjectPlaneTodoNotReadyDoesNotCallProvider(t *testing.T) { + f := &fakeProvider{} + in := readyInput() + in.Candidate.Ready = false + err := ProjectPlaneTodo(context.Background(), f, in) + if !errors.Is(err, ErrNotReady) { + t.Fatalf("expected ErrNotReady, got %v", err) + } + if len(f.calls) != 0 { + t.Fatalf("provider must not be called when not ready: %v", f.calls) + } +} + +func TestProjectPlaneTodoRejectsMissingInput(t *testing.T) { + f := &fakeProvider{} + in := readyInput() + in.OriginalBody = " " + err := ProjectPlaneTodo(context.Background(), f, in) + if !errors.Is(err, ErrInvalidProjectionInput) { + t.Fatalf("expected ErrInvalidProjectionInput, got %v", err) + } + if len(f.calls) != 0 { + t.Fatalf("provider must not be called on invalid input: %v", f.calls) + } +} diff --git a/services/core/internal/roadmapsync/retry.go b/services/core/internal/roadmapsync/retry.go new file mode 100644 index 0000000..a83407c --- /dev/null +++ b/services/core/internal/roadmapsync/retry.go @@ -0,0 +1,173 @@ +package roadmapsync + +import "strings" + +// Step is the provider-neutral creation-cycle step the retry decision resumes +// from. The values mirror the persisted step ledger so a caller can read the +// ledger and pass the completed set in without re-mapping strings. +type Step string + +const ( + StepDevelopMatched Step = "develop_matched" + StepOriginalCommentPreserved Step = "original_comment_preserved" + StepPlaneBodyUpdated Step = "plane_body_updated" + StepPlaneTodoMoved Step = "plane_todo_moved" +) + +// OrderedSteps is the creation-cycle order. A reconcile resumes at the first +// step not present in the completed set. +var OrderedSteps = []Step{ + StepDevelopMatched, + StepOriginalCommentPreserved, + StepPlaneBodyUpdated, + StepPlaneTodoMoved, +} + +// Action is the next thing a creation cycle should do after reconciling the +// persisted identity and step ledger against the current trigger. +type Action string + +const ( + // ActionResume runs the remaining steps starting at NextStep. + ActionResume Action = "resume" + // ActionComplete means every step is already recorded; nothing to do. + ActionComplete Action = "complete" + // ActionConflict means the trigger disagrees with the persisted identity + // (revision mismatch) and a human/upstream decision is required instead of a + // silent overwrite. + ActionConflict Action = "conflict" + // ActionSkipSelfMutation means the trigger was NomadCode's own mutation, so + // it must not be treated as a new authoring trigger. + ActionSkipSelfMutation Action = "skip_self_mutation" +) + +// ExistingIdentity is the provider-neutral view of a persisted identity row the +// reconcile needs: the stored identity plus the revisions it was last written +// with. Found is false when no identity is persisted yet. +type ExistingIdentity struct { + Found bool + Identity Identity + ProviderRevision string + RoadmapRevision string +} + +// ReconcileInput carries the current trigger and the persisted state to +// reconcile it against. Existing and CompletedSteps come from the identity map +// and step ledger; an empty CompletedSteps means a fresh cycle. +type ReconcileInput struct { + // Identity is the trigger's provider-neutral identity (the Plane ticket or + // Milestone path being re-processed). + Identity Identity + // ProviderRevision / RoadmapRevision are the revisions the current trigger + // observed. A mismatch against the persisted revisions is a conflict. + ProviderRevision string + RoadmapRevision string + + // Actor is who caused this trigger. When it equals SelfActor the trigger is + // NomadCode's own mutation and is not a new authoring trigger. + Actor string + SelfActor string + + Existing ExistingIdentity + CompletedSteps map[Step]bool +} + +// ReconcileResult is the typed decision: what to do next, the resume step when +// ActionResume, whether an existing identity was reused, and a reason that is +// always populated so a not-resume decision is never silent. +type ReconcileResult struct { + Action Action + NextStep Step + IdentityKept bool + Reason string +} + +// Reasons for a reconcile decision. Stable strings so a caller can branch on +// the cause. +const ( + reasonInvalidReconcileIdentity = "trigger identity is invalid or incomplete" + reasonIdentityKeyMismatch = "persisted identity key does not match trigger (provider/tenant/project/work item/path)" + reasonSelfMutation = "trigger actor is NomadCode self mutation, not a new authoring trigger" + reasonProviderRevMismatch = "trigger provider revision does not match persisted identity" + reasonRoadmapRevMismatch = "trigger roadmap revision does not match persisted identity" + reasonAllStepsComplete = "all creation cycle steps already completed" + reasonResumeRemaining = "resuming creation cycle at first incomplete step" +) + +// ReconcileCreationCycle decides the next action for a (possibly re-processed) +// creation cycle. It is the idempotency join described by plan 05: it reuses an +// existing identity when the same Plane ticket or Milestone path is +// re-processed, refuses to overwrite on a revision mismatch, skips NomadCode's +// own mutations, and otherwise resumes at the first step not yet in the ledger. +// +// The function reads no DB itself; callers supply the persisted identity and +// completed-step set so the decision stays provider-neutral and unit-testable. +func ReconcileCreationCycle(in ReconcileInput) ReconcileResult { + identity, err := in.Identity.Normalize() + if err != nil { + return ReconcileResult{Action: ActionConflict, Reason: reasonInvalidReconcileIdentity} + } + + // Actor guard: a mutation NomadCode itself made (e.g. the body/title rewrite + // it just projected) must not be re-interpreted as a fresh user trigger. + self := strings.TrimSpace(in.SelfActor) + if self != "" && strings.TrimSpace(in.Actor) == self { + return ReconcileResult{Action: ActionSkipSelfMutation, Reason: reasonSelfMutation} + } + + // Strict match: when a persisted identity exists it must agree on the full + // provider-side key (provider, tenant, project, work item id, Milestone + // path). A found-but-not-strictly-matching row means the trigger disagrees + // with what is stored (e.g. a different Plane workspace/project for the same + // work item id/path), which is a conflict rather than a silent fresh resume. + if in.Existing.Found && !in.Existing.Identity.MatchesStrict(identity) { + return ReconcileResult{Action: ActionConflict, Reason: reasonIdentityKeyMismatch} + } + identityKept := in.Existing.Found + + // Revision guard: when an identity already exists for this work item/path, + // the trigger must observe the same revisions, otherwise resuming would + // silently overwrite work projected from a different develop state. + if identityKept { + if mismatch(in.Existing.ProviderRevision, in.ProviderRevision) { + return ReconcileResult{Action: ActionConflict, IdentityKept: true, Reason: reasonProviderRevMismatch} + } + if mismatch(in.Existing.RoadmapRevision, in.RoadmapRevision) { + return ReconcileResult{Action: ActionConflict, IdentityKept: true, Reason: reasonRoadmapRevMismatch} + } + } + + next, ok := firstIncompleteStep(in.CompletedSteps) + if !ok { + return ReconcileResult{Action: ActionComplete, IdentityKept: identityKept, Reason: reasonAllStepsComplete} + } + return ReconcileResult{ + Action: ActionResume, + NextStep: next, + IdentityKept: identityKept, + Reason: reasonResumeRemaining, + } +} + +// firstIncompleteStep returns the earliest ordered step not present in the +// completed set. ok is false when every step is complete. +func firstIncompleteStep(completed map[Step]bool) (Step, bool) { + for _, step := range OrderedSteps { + if !completed[step] { + return step, true + } + } + return "", false +} + +// mismatch reports whether a persisted revision and a trigger revision disagree. +// Empty values are treated as "unknown" and never conflict, so a first cycle +// that has not recorded a revision is not blocked. +func mismatch(persisted, trigger string) bool { + p := strings.TrimSpace(persisted) + t := strings.TrimSpace(trigger) + if p == "" || t == "" { + return false + } + return p != t +} diff --git a/services/core/internal/roadmapsync/retry_test.go b/services/core/internal/roadmapsync/retry_test.go new file mode 100644 index 0000000..bc466b0 --- /dev/null +++ b/services/core/internal/roadmapsync/retry_test.go @@ -0,0 +1,184 @@ +package roadmapsync + +import ( + "testing" + + "github.com/nomadcode/nomadcode-core/internal/workitem" +) + +func reconcileIdentity() Identity { + return Identity{ + Shape: ShapeMilestone, + RoadmapMilestonePath: "agent-roadmap/phase/p/milestones/m.md", + Provider: workitem.ProviderID("plane"), + Tenant: "general", + Project: "NOMAD", + WorkItemID: "NOMAD-13", + } +} + +func TestReconcile_SamePlaneTicketReusesIdentity(t *testing.T) { + id := reconcileIdentity() + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + Existing: ExistingIdentity{Found: true, Identity: id}, + CompletedSteps: map[Step]bool{ + StepDevelopMatched: true, + }, + }) + if res.Action != ActionResume { + t.Fatalf("action: got %q, want resume (%s)", res.Action, res.Reason) + } + if !res.IdentityKept { + t.Error("expected identity reuse for the same Plane ticket") + } + if res.NextStep != StepOriginalCommentPreserved { + t.Errorf("next step: got %q, want original_comment_preserved", res.NextStep) + } +} + +func TestReconcile_SameMilestonePathReusesIdentity(t *testing.T) { + id := reconcileIdentity() + // Existing row found via the Milestone path: same path + provider work item. + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + Existing: ExistingIdentity{Found: true, Identity: id}, + CompletedSteps: map[Step]bool{}, + }) + if res.Action != ActionResume || !res.IdentityKept { + t.Fatalf("expected resume with identity kept, got %q kept=%v (%s)", res.Action, res.IdentityKept, res.Reason) + } + if res.NextStep != StepDevelopMatched { + t.Errorf("fresh ledger should resume at develop_matched, got %q", res.NextStep) + } +} + +func TestReconcile_ProviderRevisionMismatchConflicts(t *testing.T) { + id := reconcileIdentity() + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + ProviderRevision: "rev-2", + Existing: ExistingIdentity{ + Found: true, Identity: id, ProviderRevision: "rev-1", + }, + }) + if res.Action != ActionConflict { + t.Fatalf("action: got %q, want conflict (%s)", res.Action, res.Reason) + } + if !res.IdentityKept { + t.Error("a conflict on a matched identity should still report the identity as kept") + } +} + +func TestReconcile_RoadmapRevisionMismatchConflicts(t *testing.T) { + id := reconcileIdentity() + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + RoadmapRevision: "abc", + Existing: ExistingIdentity{ + Found: true, Identity: id, RoadmapRevision: "def", + }, + }) + if res.Action != ActionConflict { + t.Fatalf("action: got %q, want conflict (%s)", res.Action, res.Reason) + } +} + +func TestReconcile_NomadCodeActorIsNotNewTrigger(t *testing.T) { + id := reconcileIdentity() + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + Actor: "nomadcode", + SelfActor: "nomadcode", + Existing: ExistingIdentity{Found: true, Identity: id}, + }) + if res.Action != ActionSkipSelfMutation { + t.Fatalf("action: got %q, want skip_self_mutation (%s)", res.Action, res.Reason) + } +} + +func TestReconcile_DifferentTenantConflicts(t *testing.T) { + id := reconcileIdentity() + existing := id + existing.Tenant = "other-workspace" + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + Existing: ExistingIdentity{Found: true, Identity: existing}, + CompletedSteps: map[Step]bool{}, + }) + if res.Action != ActionConflict { + t.Fatalf("tenant mismatch should conflict, got %q (%s)", res.Action, res.Reason) + } + if res.IdentityKept { + t.Error("a non-matching persisted identity must not be reported as kept") + } +} + +func TestReconcile_DifferentProjectConflicts(t *testing.T) { + id := reconcileIdentity() + existing := id + existing.Project = "OTHER" + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + Existing: ExistingIdentity{Found: true, Identity: existing}, + CompletedSteps: map[Step]bool{}, + }) + if res.Action != ActionConflict { + t.Fatalf("project mismatch should conflict, got %q (%s)", res.Action, res.Reason) + } +} + +func TestReconcile_AllStepsCompleteReturnsComplete(t *testing.T) { + id := reconcileIdentity() + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + Existing: ExistingIdentity{Found: true, Identity: id}, + CompletedSteps: map[Step]bool{ + StepDevelopMatched: true, + StepOriginalCommentPreserved: true, + StepPlaneBodyUpdated: true, + StepPlaneTodoMoved: true, + }, + }) + if res.Action != ActionComplete { + t.Fatalf("action: got %q, want complete (%s)", res.Action, res.Reason) + } +} + +func TestReconcile_InvalidIdentityConflicts(t *testing.T) { + res := ReconcileCreationCycle(ReconcileInput{ + Identity: Identity{Provider: workitem.ProviderID("plane")}, // missing path + work item id + }) + if res.Action != ActionConflict { + t.Fatalf("action: got %q, want conflict (%s)", res.Action, res.Reason) + } +} + +func TestReconcile_FreshCycleNoExistingResumesAtFirstStep(t *testing.T) { + res := ReconcileCreationCycle(ReconcileInput{ + Identity: reconcileIdentity(), + Existing: ExistingIdentity{Found: false}, + CompletedSteps: map[Step]bool{}, + }) + if res.Action != ActionResume || res.NextStep != StepDevelopMatched { + t.Fatalf("fresh cycle should resume at develop_matched, got %q step=%q", res.Action, res.NextStep) + } + if res.IdentityKept { + t.Error("no existing identity should report IdentityKept=false") + } +} + +func TestReconcile_FirstCycleRevisionUnknownDoesNotConflict(t *testing.T) { + id := reconcileIdentity() + // Existing identity has no recorded revision yet; a trigger that carries one + // must not be blocked. + res := ReconcileCreationCycle(ReconcileInput{ + Identity: id, + ProviderRevision: "rev-1", + Existing: ExistingIdentity{Found: true, Identity: id, ProviderRevision: ""}, + CompletedSteps: map[Step]bool{}, + }) + if res.Action != ActionResume { + t.Fatalf("unknown persisted revision should not conflict, got %q (%s)", res.Action, res.Reason) + } +} diff --git a/services/core/internal/storage/roadmap_sync_identities.go b/services/core/internal/storage/roadmap_sync_identities.go new file mode 100644 index 0000000..875b694 --- /dev/null +++ b/services/core/internal/storage/roadmap_sync_identities.go @@ -0,0 +1,84 @@ +package storage + +import ( + "context" + "errors" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/nomadcode/nomadcode-core/internal/db" +) + +// pgUniqueViolation is the SQLSTATE code Postgres returns for a unique +// constraint violation. +const pgUniqueViolation = "23505" + +// ErrRoadmapSyncIdentityNotFound is returned when no identity row matches the +// lookup key. +var ErrRoadmapSyncIdentityNotFound = errors.New("roadmap sync identity not found") + +// ErrRoadmapSyncIdentityConflict is returned when the incoming provider work +// item and Milestone path do not agree with an existing identity row: either +// the Milestone path is already bound to a different provider work item, or the +// provider work item is already bound to a different Milestone path. Returning a +// typed conflict keeps a re-process from silently rebinding one identity onto +// another instead of hitting a raw unique-index violation. +var ErrRoadmapSyncIdentityConflict = errors.New("roadmap sync identity conflict") + +// GetRoadmapSyncIdentityByWorkItem returns the identity bound to a provider work +// item, or ErrRoadmapSyncIdentityNotFound when none exists. +func (s *Store) GetRoadmapSyncIdentityByWorkItem(ctx context.Context, args db.GetRoadmapSyncIdentityByWorkItemParams) (db.RoadmapSyncIdentity, error) { + row, err := s.queries.GetRoadmapSyncIdentityByWorkItem(ctx, args) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return db.RoadmapSyncIdentity{}, ErrRoadmapSyncIdentityNotFound + } + return db.RoadmapSyncIdentity{}, err + } + return row, nil +} + +// GetRoadmapSyncIdentityByMilestonePath returns the identity bound to a roadmap +// Milestone path, or ErrRoadmapSyncIdentityNotFound when none exists. +func (s *Store) GetRoadmapSyncIdentityByMilestonePath(ctx context.Context, milestonePath string) (db.RoadmapSyncIdentity, error) { + row, err := s.queries.GetRoadmapSyncIdentityByMilestonePath(ctx, milestonePath) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return db.RoadmapSyncIdentity{}, ErrRoadmapSyncIdentityNotFound + } + return db.RoadmapSyncIdentity{}, err + } + return row, nil +} + +// UpsertRoadmapSyncIdentity persists an identity keyed on the provider work +// item, reusing the existing row when the same Plane ticket or the same +// Milestone path is re-processed. +// +// Conflict protection is atomic in a single statement rather than a non-atomic +// read-then-write: the upsert's DO UPDATE keeps the stored roadmap_milestone_path +// and only fires when it already equals the incoming path. So a re-process of +// the same provider work item with a different Milestone path matches no row on +// conflict and returns pgx.ErrNoRows, which maps to ErrRoadmapSyncIdentityConflict +// instead of silently rebinding the ticket to a new path. A different work item +// re-using an existing Milestone path trips the path unique index (SQLSTATE +// 23505), which also maps to ErrRoadmapSyncIdentityConflict instead of a raw pg +// error. Two concurrent re-processes therefore cannot both win. +func (s *Store) UpsertRoadmapSyncIdentity(ctx context.Context, args db.UpsertRoadmapSyncIdentityByWorkItemParams) (db.RoadmapSyncIdentity, error) { + row, err := s.queries.UpsertRoadmapSyncIdentityByWorkItem(ctx, args) + if err != nil { + // Work item already bound to a different Milestone path: the conditional + // DO UPDATE matched no row, so RETURNING produced nothing. + if errors.Is(err, pgx.ErrNoRows) { + return db.RoadmapSyncIdentity{}, ErrRoadmapSyncIdentityConflict + } + // Different work item re-using an existing Milestone path: the path unique + // index rejected the INSERT. + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) && pgErr.Code == pgUniqueViolation { + return db.RoadmapSyncIdentity{}, ErrRoadmapSyncIdentityConflict + } + return db.RoadmapSyncIdentity{}, err + } + return row, nil +} diff --git a/services/core/internal/storage/roadmap_sync_identities_test.go b/services/core/internal/storage/roadmap_sync_identities_test.go new file mode 100644 index 0000000..79dbfcc --- /dev/null +++ b/services/core/internal/storage/roadmap_sync_identities_test.go @@ -0,0 +1,249 @@ +package storage + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/nomadcode/nomadcode-core/internal/db" +) + +// identityRouteResult is the row/err a routed QueryRow should produce. +type identityRouteResult struct { + row db.RoadmapSyncIdentity + err error +} + +// identityMockDBTX routes QueryRow calls by a substring found in the SQL so a +// single mock can answer the by-path lookup, the by-work-item lookup, and the +// upsert with different rows. +type identityMockDBTX struct { + byPath identityRouteResult + byWork identityRouteResult + upsert identityRouteResult + queryLog []string +} + +func (m *identityMockDBTX) Exec(ctx context.Context, sql string, args ...interface{}) (pgconn.CommandTag, error) { + return pgconn.NewCommandTag("OK"), nil +} + +func (m *identityMockDBTX) Query(ctx context.Context, sql string, args ...interface{}) (pgx.Rows, error) { + return nil, errors.New("unexpected Query call") +} + +func (m *identityMockDBTX) QueryRow(ctx context.Context, sql string, args ...interface{}) pgx.Row { + m.queryLog = append(m.queryLog, sql) + switch { + case strings.Contains(sql, "INSERT INTO roadmap_sync_identities"): + return &identityMockRow{result: m.upsert} + case strings.Contains(sql, "WHERE roadmap_milestone_path = $1"): + return &identityMockRow{result: m.byPath} + case strings.Contains(sql, "WHERE provider = $1"): + return &identityMockRow{result: m.byWork} + default: + return &identityMockRow{result: identityRouteResult{err: errors.New("unrouted sql: " + sql)}} + } +} + +type identityMockRow struct { + result identityRouteResult +} + +func (r *identityMockRow) Scan(dest ...interface{}) error { + if r.result.err != nil { + return r.result.err + } + scanRoadmapSyncIdentity(r.result.row, dest) + return nil +} + +// scanRoadmapSyncIdentity fills scan destinations in the column order used by +// the generated roadmap_sync_identities queries. +func scanRoadmapSyncIdentity(row db.RoadmapSyncIdentity, dest []interface{}) { + if len(dest) < 13 { + return + } + assign(dest[0], row.ID) + assign(dest[1], row.Shape) + assign(dest[2], row.RoadmapMilestonePath) + assign(dest[3], row.RoadmapItemID) + assign(dest[4], row.Provider) + assign(dest[5], row.Tenant) + assign(dest[6], row.Project) + assign(dest[7], row.WorkItemID) + assign(dest[8], row.ParentWorkItemID) + if p, ok := dest[9].(**string); ok { + *p = row.ProviderRevision + } + if p, ok := dest[10].(**string); ok { + *p = row.RoadmapRevision + } + if p, ok := dest[11].(*time.Time); ok { + *p = row.CreatedAt + } + if p, ok := dest[12].(*time.Time); ok { + *p = row.UpdatedAt + } +} + +func assign[T any](dst interface{}, v T) { + if p, ok := dst.(*T); ok { + *p = v + } +} + +func newIdentityStore(m *identityMockDBTX) *Store { + return NewStoreWithQueries(db.New(m)) +} + +func sampleUpsertParams() db.UpsertRoadmapSyncIdentityByWorkItemParams { + return db.UpsertRoadmapSyncIdentityByWorkItemParams{ + Shape: "milestone", + RoadmapMilestonePath: "agent-roadmap/phase/p/milestones/m.md", + Provider: "plane", + Tenant: "general", + Project: "NOMAD", + WorkItemID: "NOMAD-13", + } +} + +// ---- UpsertRoadmapSyncIdentity ---- + +func TestUpsertRoadmapSyncIdentity_ByWorkItemReturnsRow(t *testing.T) { + args := sampleUpsertParams() + m := &identityMockDBTX{ + upsert: identityRouteResult{row: db.RoadmapSyncIdentity{ + ID: 7, Shape: "milestone", RoadmapMilestonePath: args.RoadmapMilestonePath, + Provider: "plane", Tenant: "general", Project: "NOMAD", WorkItemID: "NOMAD-13", + }}, + } + store := newIdentityStore(m) + + row, err := store.UpsertRoadmapSyncIdentity(context.Background(), args) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if row.ID != 7 || row.WorkItemID != "NOMAD-13" || row.RoadmapMilestonePath != args.RoadmapMilestonePath { + t.Errorf("identity not preserved: %+v", row) + } + if !sqlLogContains(m.queryLog, "ON CONFLICT (provider, tenant, project, work_item_id)") { + t.Errorf("expected upsert query shape, log=%v", m.queryLog) + } + // The conflict path must keep the stored Milestone path and only update when + // the incoming path already matches, so the rebind race is closed in SQL. + if !sqlLogContains(m.queryLog, "WHERE roadmap_sync_identities.roadmap_milestone_path = EXCLUDED.roadmap_milestone_path") { + t.Errorf("expected conditional path guard in DO UPDATE, log=%v", m.queryLog) + } + if sqlLogContains(m.queryLog, "roadmap_milestone_path = EXCLUDED.roadmap_milestone_path,") { + t.Errorf("DO UPDATE must not overwrite roadmap_milestone_path, log=%v", m.queryLog) + } +} + +func TestUpsertRoadmapSyncIdentity_SamePathSameWorkItemReuses(t *testing.T) { + args := sampleUpsertParams() + existing := db.RoadmapSyncIdentity{ + ID: 7, Shape: "milestone", RoadmapMilestonePath: args.RoadmapMilestonePath, + Provider: "plane", Tenant: "general", Project: "NOMAD", WorkItemID: "NOMAD-13", + } + m := &identityMockDBTX{upsert: identityRouteResult{row: existing}} + store := newIdentityStore(m) + + row, err := store.UpsertRoadmapSyncIdentity(context.Background(), args) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if row.ID != 7 { + t.Errorf("expected reuse of row 7, got %+v", row) + } +} + +// Same provider work item arriving with a different Milestone path: the +// conditional DO UPDATE matches no row, RETURNING is empty, and the store maps +// pgx.ErrNoRows to a typed conflict instead of rebinding the ticket. +func TestUpsertRoadmapSyncIdentity_WorkItemRebindToDifferentPathConflicts(t *testing.T) { + args := sampleUpsertParams() + m := &identityMockDBTX{upsert: identityRouteResult{err: pgx.ErrNoRows}} + store := newIdentityStore(m) + + _, err := store.UpsertRoadmapSyncIdentity(context.Background(), args) + if !errors.Is(err, ErrRoadmapSyncIdentityConflict) { + t.Fatalf("expected conflict, got %v", err) + } +} + +// A different provider work item re-using an existing Milestone path trips the +// path unique index (SQLSTATE 23505); the store maps it to a typed conflict +// rather than surfacing a raw pg error. +func TestUpsertRoadmapSyncIdentity_PathUniqueViolationConflicts(t *testing.T) { + args := sampleUpsertParams() + m := &identityMockDBTX{upsert: identityRouteResult{err: &pgconn.PgError{ + Code: pgUniqueViolation, + ConstraintName: "ux_roadmap_sync_identities_milestone_path", + }}} + store := newIdentityStore(m) + + _, err := store.UpsertRoadmapSyncIdentity(context.Background(), args) + if !errors.Is(err, ErrRoadmapSyncIdentityConflict) { + t.Fatalf("expected conflict, got %v", err) + } +} + +// A non-conflict DB error must propagate unchanged, not be swallowed as a +// conflict. +func TestUpsertRoadmapSyncIdentity_OtherDBErrorPropagates(t *testing.T) { + args := sampleUpsertParams() + sentinel := errors.New("connection reset") + m := &identityMockDBTX{upsert: identityRouteResult{err: sentinel}} + store := newIdentityStore(m) + + _, err := store.UpsertRoadmapSyncIdentity(context.Background(), args) + if !errors.Is(err, sentinel) { + t.Fatalf("expected sentinel error, got %v", err) + } + if errors.Is(err, ErrRoadmapSyncIdentityConflict) { + t.Error("generic DB error must not be reported as conflict") + } +} + +// ---- Get lookups ---- + +func TestGetRoadmapSyncIdentityByWorkItem_NotFoundSentinel(t *testing.T) { + m := &identityMockDBTX{byWork: identityRouteResult{err: pgx.ErrNoRows}} + store := newIdentityStore(m) + + _, err := store.GetRoadmapSyncIdentityByWorkItem(context.Background(), db.GetRoadmapSyncIdentityByWorkItemParams{ + Provider: "plane", Tenant: "general", Project: "NOMAD", WorkItemID: "NOMAD-13", + }) + if !errors.Is(err, ErrRoadmapSyncIdentityNotFound) { + t.Fatalf("expected not-found sentinel, got %v", err) + } +} + +func TestGetRoadmapSyncIdentityByMilestonePath_ReturnsRow(t *testing.T) { + m := &identityMockDBTX{byPath: identityRouteResult{row: db.RoadmapSyncIdentity{ + ID: 4, RoadmapMilestonePath: "agent-roadmap/phase/p/milestones/m.md", WorkItemID: "NOMAD-13", + }}} + store := newIdentityStore(m) + + row, err := store.GetRoadmapSyncIdentityByMilestonePath(context.Background(), "agent-roadmap/phase/p/milestones/m.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if row.ID != 4 || row.WorkItemID != "NOMAD-13" { + t.Errorf("row not preserved: %+v", row) + } +} + +func sqlLogContains(logs []string, sub string) bool { + for _, l := range logs { + if strings.Contains(l, sub) { + return true + } + } + return false +} diff --git a/services/core/internal/storage/roadmap_sync_steps.go b/services/core/internal/storage/roadmap_sync_steps.go new file mode 100644 index 0000000..224d2a4 --- /dev/null +++ b/services/core/internal/storage/roadmap_sync_steps.go @@ -0,0 +1,72 @@ +package storage + +import ( + "context" + "errors" + + "github.com/nomadcode/nomadcode-core/internal/db" +) + +// RoadmapSyncStep is the closed set of creation-cycle steps tracked in the +// ledger, in execution order. A completed step is skipped on re-process so a +// partially-failed cycle resumes only the remaining steps. +type RoadmapSyncStep string + +const ( + StepDevelopMatched RoadmapSyncStep = "develop_matched" + StepOriginalCommentPreserved RoadmapSyncStep = "original_comment_preserved" + StepPlaneBodyUpdated RoadmapSyncStep = "plane_body_updated" + StepPlaneTodoMoved RoadmapSyncStep = "plane_todo_moved" +) + +// OrderedRoadmapSyncSteps lists the steps in the order the creation cycle runs +// them, so a resume can pick the first not-yet-completed step. +var OrderedRoadmapSyncSteps = []RoadmapSyncStep{ + StepDevelopMatched, + StepOriginalCommentPreserved, + StepPlaneBodyUpdated, + StepPlaneTodoMoved, +} + +// ErrUnknownRoadmapSyncStep is returned when a caller marks or queries a step +// outside the closed set, mirroring the DB check constraint so a typo never +// silently records an unknown step. +var ErrUnknownRoadmapSyncStep = errors.New("unknown roadmap sync step") + +// Valid reports whether the step is part of the tracked closed set. +func (s RoadmapSyncStep) Valid() bool { + switch s { + case StepDevelopMatched, StepOriginalCommentPreserved, StepPlaneBodyUpdated, StepPlaneTodoMoved: + return true + default: + return false + } +} + +// MarkStepCompleted records that a step finished for an identity. Re-marking an +// already-completed step is a no-op that returns the existing row, so a retry +// after a later-step failure never duplicates ledger rows. An unknown step is +// rejected before touching the DB. +func (s *Store) MarkStepCompleted(ctx context.Context, identityID int64, step RoadmapSyncStep) (db.RoadmapSyncStep, error) { + if !step.Valid() { + return db.RoadmapSyncStep{}, ErrUnknownRoadmapSyncStep + } + return s.queries.MarkRoadmapSyncStepCompleted(ctx, db.MarkRoadmapSyncStepCompletedParams{ + RoadmapSyncIdentityID: identityID, + Step: string(step), + }) +} + +// CompletedSteps returns the set of steps already completed for an identity, so +// a retry decision can skip them. +func (s *Store) CompletedSteps(ctx context.Context, identityID int64) (map[RoadmapSyncStep]bool, error) { + rows, err := s.queries.ListRoadmapSyncSteps(ctx, identityID) + if err != nil { + return nil, err + } + done := make(map[RoadmapSyncStep]bool, len(rows)) + for _, r := range rows { + done[RoadmapSyncStep(r.Step)] = true + } + return done, nil +} diff --git a/services/core/internal/storage/roadmap_sync_steps_test.go b/services/core/internal/storage/roadmap_sync_steps_test.go new file mode 100644 index 0000000..d39261f --- /dev/null +++ b/services/core/internal/storage/roadmap_sync_steps_test.go @@ -0,0 +1,180 @@ +package storage + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/nomadcode/nomadcode-core/internal/db" +) + +// stepMockDBTX answers the ledger list (Query) and mark (QueryRow) calls. +type stepMockDBTX struct { + listRows []db.RoadmapSyncStep + listErr error + markRow db.RoadmapSyncStep + markErr error + + queryLog []string + markArgs []interface{} +} + +func (m *stepMockDBTX) Exec(ctx context.Context, sql string, args ...interface{}) (pgconn.CommandTag, error) { + return pgconn.NewCommandTag("OK"), nil +} + +func (m *stepMockDBTX) Query(ctx context.Context, sql string, args ...interface{}) (pgx.Rows, error) { + m.queryLog = append(m.queryLog, sql) + if m.listErr != nil { + return nil, m.listErr + } + return &fakeStepRows{rows: m.listRows}, nil +} + +func (m *stepMockDBTX) QueryRow(ctx context.Context, sql string, args ...interface{}) pgx.Row { + m.queryLog = append(m.queryLog, sql) + m.markArgs = args + return &stepMockRow{row: m.markRow, err: m.markErr} +} + +type stepMockRow struct { + row db.RoadmapSyncStep + err error +} + +func (r *stepMockRow) Scan(dest ...interface{}) error { + if r.err != nil { + return r.err + } + scanRoadmapSyncStep(r.row, dest) + return nil +} + +type fakeStepRows struct { + rows []db.RoadmapSyncStep + idx int +} + +func (r *fakeStepRows) Close() {} +func (r *fakeStepRows) Err() error { return nil } +func (r *fakeStepRows) CommandTag() pgconn.CommandTag { return pgconn.NewCommandTag("SELECT") } +func (r *fakeStepRows) FieldDescriptions() []pgconn.FieldDescription { return nil } +func (r *fakeStepRows) Values() ([]interface{}, error) { return nil, nil } +func (r *fakeStepRows) RawValues() [][]byte { return nil } +func (r *fakeStepRows) Conn() *pgx.Conn { return nil } + +func (r *fakeStepRows) Next() bool { + if r.idx >= len(r.rows) { + return false + } + r.idx++ + return true +} + +func (r *fakeStepRows) Scan(dest ...interface{}) error { + scanRoadmapSyncStep(r.rows[r.idx-1], dest) + return nil +} + +func scanRoadmapSyncStep(row db.RoadmapSyncStep, dest []interface{}) { + if len(dest) < 4 { + return + } + assign(dest[0], row.ID) + assign(dest[1], row.RoadmapSyncIdentityID) + assign(dest[2], row.Step) + if p, ok := dest[3].(*time.Time); ok { + *p = row.CompletedAt + } +} + +func newStepStore(m *stepMockDBTX) *Store { + return NewStoreWithQueries(db.New(m)) +} + +// ---- MarkStepCompleted ---- + +func TestMarkStepCompleted_KnownStepPersists(t *testing.T) { + completedAt := time.Now() + m := &stepMockDBTX{markRow: db.RoadmapSyncStep{ + ID: 1, RoadmapSyncIdentityID: 7, Step: string(StepDevelopMatched), CompletedAt: completedAt, + }} + store := newStepStore(m) + + row, err := store.MarkStepCompleted(context.Background(), 7, StepDevelopMatched) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if row.Step != string(StepDevelopMatched) || row.RoadmapSyncIdentityID != 7 { + t.Errorf("row not preserved: %+v", row) + } + if !sqlLogContains(m.queryLog, "ON CONFLICT (roadmap_sync_identity_id, step)") { + t.Errorf("expected idempotent upsert shape, log=%v", m.queryLog) + } +} + +func TestMarkStepCompleted_CompletedStepReapplyIsNoOp(t *testing.T) { + // The DB upsert returns the original row (same completed_at) on re-mark; the + // store passes it through without error so a re-apply is a no-op. + original := time.Date(2026, 6, 13, 10, 0, 0, 0, time.UTC) + m := &stepMockDBTX{markRow: db.RoadmapSyncStep{ + ID: 1, RoadmapSyncIdentityID: 7, Step: string(StepPlaneBodyUpdated), CompletedAt: original, + }} + store := newStepStore(m) + + row, err := store.MarkStepCompleted(context.Background(), 7, StepPlaneBodyUpdated) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !row.CompletedAt.Equal(original) { + t.Errorf("re-apply must keep original completed_at, got %v", row.CompletedAt) + } +} + +func TestMarkStepCompleted_UnknownStepRejected(t *testing.T) { + m := &stepMockDBTX{} + store := newStepStore(m) + + _, err := store.MarkStepCompleted(context.Background(), 7, RoadmapSyncStep("plane_done_moved")) + if !errors.Is(err, ErrUnknownRoadmapSyncStep) { + t.Fatalf("expected unknown-step error, got %v", err) + } + if len(m.queryLog) != 0 { + t.Errorf("unknown step must not touch the DB, log=%v", m.queryLog) + } +} + +// ---- CompletedSteps ---- + +func TestCompletedSteps_ReturnsSetForResume(t *testing.T) { + m := &stepMockDBTX{listRows: []db.RoadmapSyncStep{ + {ID: 1, RoadmapSyncIdentityID: 7, Step: string(StepDevelopMatched)}, + {ID: 2, RoadmapSyncIdentityID: 7, Step: string(StepOriginalCommentPreserved)}, + }} + store := newStepStore(m) + + done, err := store.CompletedSteps(context.Background(), 7) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !done[StepDevelopMatched] || !done[StepOriginalCommentPreserved] { + t.Errorf("expected first two steps completed, got %v", done) + } + if done[StepPlaneBodyUpdated] || done[StepPlaneTodoMoved] { + t.Errorf("uncompleted steps must be absent, got %v", done) + } +} + +func TestCompletedSteps_PropagatesQueryError(t *testing.T) { + sentinel := errors.New("query failed") + m := &stepMockDBTX{listErr: sentinel} + store := newStepStore(m) + + _, err := store.CompletedSteps(context.Background(), 7) + if !errors.Is(err, sentinel) { + t.Fatalf("expected sentinel error, got %v", err) + } +} diff --git a/services/core/internal/workitem/provider.go b/services/core/internal/workitem/provider.go index 4a9861a..f935822 100644 --- a/services/core/internal/workitem/provider.go +++ b/services/core/internal/workitem/provider.go @@ -21,6 +21,7 @@ const ( CapabilityStatusProjection Capability = "status_projection" CapabilityLabelProjection Capability = "label_projection" CapabilityCreate Capability = "create" + CapabilityBodyProjection Capability = "body_projection" ) type Capabilities map[Capability]bool @@ -50,6 +51,13 @@ type Creator interface { CreateWorkItem(ctx context.Context, input CreateInput) (CreateResult, error) } +// BodyProjector updates an existing work item's title and/or body. It is an +// additive projection capability separate from status projection so a body +// rewrite and a status move stay independent steps. +type BodyProjector interface { + ProjectBody(ctx context.Context, input BodyProjection) error +} + type LabelProjector interface { ProjectLabels(ctx context.Context, input LabelProjection) error } @@ -97,6 +105,12 @@ type StatusProjection struct { ProviderState string `json:"provider_state"` } +type BodyProjection struct { + Ref Ref `json:"ref"` + Title string `json:"title,omitempty"` + DescriptionHTML string `json:"description_html,omitempty"` +} + type CreateInput struct { Provider ProviderID `json:"provider"` Tenant string `json:"tenant"` diff --git a/services/core/migrations/00006_create_roadmap_sync_identities.sql b/services/core/migrations/00006_create_roadmap_sync_identities.sql new file mode 100644 index 0000000..d8ae44f --- /dev/null +++ b/services/core/migrations/00006_create_roadmap_sync_identities.sql @@ -0,0 +1,34 @@ +-- +goose Up +-- +goose StatementBegin +CREATE TABLE IF NOT EXISTS roadmap_sync_identities ( + id BIGSERIAL PRIMARY KEY, + shape TEXT NOT NULL DEFAULT 'milestone', + roadmap_milestone_path TEXT NOT NULL, + roadmap_item_id TEXT NOT NULL DEFAULT '', + provider TEXT NOT NULL, + tenant TEXT NOT NULL DEFAULT '', + project TEXT NOT NULL DEFAULT '', + work_item_id TEXT NOT NULL, + parent_work_item_id TEXT NOT NULL DEFAULT '', + provider_revision TEXT, + roadmap_revision TEXT, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + CONSTRAINT roadmap_sync_identities_shape_check + CHECK (shape IN ('milestone', 'task')) +); + +-- The same provider work item maps to exactly one identity row, so re-processing +-- the same Plane ticket never creates a duplicate Milestone identity. +CREATE UNIQUE INDEX ux_roadmap_sync_identities_provider_work_item + ON roadmap_sync_identities (provider, tenant, project, work_item_id); +-- The same Milestone path maps to exactly one identity row, so re-processing the +-- same Milestone path never creates a duplicate identity. +CREATE UNIQUE INDEX ux_roadmap_sync_identities_milestone_path + ON roadmap_sync_identities (roadmap_milestone_path); +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP TABLE IF EXISTS roadmap_sync_identities; +-- +goose StatementEnd diff --git a/services/core/migrations/00007_create_roadmap_sync_steps.sql b/services/core/migrations/00007_create_roadmap_sync_steps.sql new file mode 100644 index 0000000..2393d1b --- /dev/null +++ b/services/core/migrations/00007_create_roadmap_sync_steps.sql @@ -0,0 +1,21 @@ +-- +goose Up +-- +goose StatementBegin +CREATE TABLE IF NOT EXISTS roadmap_sync_steps ( + id BIGSERIAL PRIMARY KEY, + roadmap_sync_identity_id BIGINT NOT NULL REFERENCES roadmap_sync_identities(id) ON DELETE CASCADE, + step TEXT NOT NULL, + completed_at TIMESTAMPTZ NOT NULL DEFAULT now(), + CONSTRAINT roadmap_sync_steps_step_check + CHECK (step IN ('develop_matched', 'original_comment_preserved', 'plane_body_updated', 'plane_todo_moved')) +); + +-- Each identity records a given step at most once, so re-applying a completed +-- step is idempotent rather than appending a duplicate ledger row. +CREATE UNIQUE INDEX ux_roadmap_sync_steps_identity_step + ON roadmap_sync_steps (roadmap_sync_identity_id, step); +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP TABLE IF EXISTS roadmap_sync_steps; +-- +goose StatementEnd diff --git a/services/core/queries/roadmap_sync_identities.sql b/services/core/queries/roadmap_sync_identities.sql new file mode 100644 index 0000000..82ca698 --- /dev/null +++ b/services/core/queries/roadmap_sync_identities.sql @@ -0,0 +1,25 @@ +-- name: GetRoadmapSyncIdentityByWorkItem :one +SELECT id, shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision, created_at, updated_at +FROM roadmap_sync_identities +WHERE provider = $1 AND tenant = $2 AND project = $3 AND work_item_id = $4; + +-- name: GetRoadmapSyncIdentityByMilestonePath :one +SELECT id, shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision, created_at, updated_at +FROM roadmap_sync_identities +WHERE roadmap_milestone_path = $1; + +-- name: UpsertRoadmapSyncIdentityByWorkItem :one +INSERT INTO roadmap_sync_identities ( + shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision +) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) +ON CONFLICT (provider, tenant, project, work_item_id) +DO UPDATE SET + shape = EXCLUDED.shape, + roadmap_item_id = EXCLUDED.roadmap_item_id, + parent_work_item_id = EXCLUDED.parent_work_item_id, + provider_revision = EXCLUDED.provider_revision, + roadmap_revision = EXCLUDED.roadmap_revision, + updated_at = now() +WHERE roadmap_sync_identities.roadmap_milestone_path = EXCLUDED.roadmap_milestone_path +RETURNING id, shape, roadmap_milestone_path, roadmap_item_id, provider, tenant, project, work_item_id, parent_work_item_id, provider_revision, roadmap_revision, created_at, updated_at; diff --git a/services/core/queries/roadmap_sync_steps.sql b/services/core/queries/roadmap_sync_steps.sql new file mode 100644 index 0000000..551a3e2 --- /dev/null +++ b/services/core/queries/roadmap_sync_steps.sql @@ -0,0 +1,12 @@ +-- name: ListRoadmapSyncSteps :many +SELECT id, roadmap_sync_identity_id, step, completed_at +FROM roadmap_sync_steps +WHERE roadmap_sync_identity_id = $1 +ORDER BY id; + +-- name: MarkRoadmapSyncStepCompleted :one +INSERT INTO roadmap_sync_steps (roadmap_sync_identity_id, step) +VALUES ($1, $2) +ON CONFLICT (roadmap_sync_identity_id, step) +DO UPDATE SET roadmap_sync_identity_id = EXCLUDED.roadmap_sync_identity_id +RETURNING id, roadmap_sync_identity_id, step, completed_at;