From 0d619b446c024707c6e8028a16ac350966bea3a5 Mon Sep 17 00:00:00 2001 From: toki Date: Fri, 3 Jul 2026 04:19:10 +0900 Subject: [PATCH] =?UTF-8?q?feat(sync):=20roadmap=20identity=20backfill=20?= =?UTF-8?q?=EA=B8=B0=EB=8A=A5=EA=B3=BC=20git=20backfiller=EB=A5=BC=20?= =?UTF-8?q?=EC=B6=94=EA=B0=80=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit roadmap sync 과정에서 origin 프로젝트 간행본의 identity 정보를 채우는 backfill 파이프라인을 구현한다. - git_backfiller: git 레이블/태그 간행본의 identity 백필 - migration: backfill 완료 단계 테이블 스키마 추가 - pipeline/service: backfill 호출 흐름 연동 - identity_write: backfill 결과 쓰기 통합 --- .../code_review_cloud_G08_0.log | 204 +++++++++ .../code_review_cloud_G08_1.log | 248 +++++++++++ .../02+01_identity_backfill/complete.log | 49 +++ .../plan_cloud_G08_0.log} | 0 .../plan_cloud_G08_1.log | 265 ++++++++++++ .../CODE_REVIEW-cloud-G08.md | 127 ------ services/core/cmd/server/main.go | 12 + services/core/cmd/server/main_test.go | 21 + .../core/internal/authoring/identity_write.go | 45 +- services/core/internal/roadmapsync/retry.go | 2 + .../core/internal/roadmapsync/retry_test.go | 5 +- .../roadmapsyncpipeline/git_backfiller.go | 270 ++++++++++++ .../git_backfiller_test.go | 392 ++++++++++++++++++ .../internal/roadmapsyncpipeline/service.go | 88 +++- .../roadmapsyncpipeline/service_test.go | 89 +++- .../internal/storage/roadmap_sync_steps.go | 4 +- .../storage/roadmap_sync_steps_test.go | 23 +- ...0_add_roadmap_identity_backfilled_step.sql | 21 + 18 files changed, 1696 insertions(+), 169 deletions(-) create mode 100644 agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_0.log create mode 100644 agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_1.log create mode 100644 agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/complete.log rename agent-task/{m-agent-origin-milestone-creation-sync/02+01_identity_backfill/PLAN-cloud-G08.md => archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_0.log} (100%) create mode 100644 agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_1.log delete mode 100644 agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/CODE_REVIEW-cloud-G08.md create mode 100644 services/core/internal/roadmapsyncpipeline/git_backfiller.go create mode 100644 services/core/internal/roadmapsyncpipeline/git_backfiller_test.go create mode 100644 services/core/migrations/00010_add_roadmap_identity_backfilled_step.sql diff --git a/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_0.log b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_0.log new file mode 100644 index 0000000..bab92fc --- /dev/null +++ b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_0.log @@ -0,0 +1,204 @@ + + +# Code Review Reference - API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked 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. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only. + +## 개요 + +date=2026-07-02 +task=m-agent-origin-milestone-creation-sync/02+01_identity_backfill, plan=0, tag=API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/agent-origin-milestone-creation-sync.md` +- Task ids: + - `identity-backfill`: Plane 생성 직후 Milestone markdown에 `Provider identity` block을 주입하고 Core-authored backfill commit을 `develop`에 push한다. +- Completion mode: check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 판정 append, active 파일 log archive, PASS 시 `complete.log` 작성과 task directory archive까지 끝난 상태를 의미합니다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [API-1] Durable Backfill Step | [x] | +| [API-2] Identity Backfill Writer And Commit Scope | [x] | +| [API-3] Scheduler Wiring And Review Evidence | [x] | + +## 구현 체크리스트 + +- [x] `identity-backfill` step을 storage/domain ordered step set에 추가하고 DB check constraint migration/query tests를 갱신한다. +- [x] missing-create Plane create + identity upsert 직후 Milestone markdown에 `Provider identity` block을 주입하는 pipeline seam을 추가한다. +- [x] backfill writer가 identity block 외 unrelated roadmap 변경을 commit하지 않는 git scope check를 구현/테스트한다. 검증: backfill commit에는 identity block 외의 unrelated roadmap 변경이 포함되지 않는다. +- [x] push 성공 뒤에만 `identity_backfilled` ledger step을 mark하고, 실패 시 retry가 create를 반복하지 않고 backfill부터 재개하게 한다. +- [x] `git diff --check`, focused `go test -count=1`, focused `go vet`를 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G08_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_cloud_G08_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [ ] PASS이면 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [ ] PASS이면 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/`로 이동한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [x] WARN/FAIL이면 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성하고 `complete.log`를 작성하지 않는다. + +## 계획 대비 변경 사항 + +- `NewService` 시그니처는 변경하지 않고 `Service.SetBackfiller(Backfiller)` post-construction setter를 추가했다 (기존 `workflowService.SetEnqueuer` 패턴과 동일). 기존 모든 `NewService` 호출부(`main.go`, `service_test.go`의 다수 테스트)가 그대로 유지된다. +- Plan의 API-2 "After" 코드는 missing-create 최초 생성 경로만 보여줬지만, 실제로는 "이미 identity가 생성됐지만 backfill push가 아직 안 끝난 채로 같은 missing-create 트리거가 재전달"되는 재시도 경로를 별도로 처리해야 했다(체크리스트 항목 4 "실패 시 retry가 create를 반복하지 않고 backfill부터 재개"). `SyncCreation` 최상단에서 트리거의 원본 `Expected.WorkItemID`/`Ref.ID`가 둘 다 빈 값인지(`missingCreateShaped`)를 found-infill 이전에 캡처해 두고, `if found` 블록에서 `existingRow.WorkItemID != "" && !completed[identity_backfilled]`이면 재생성 없이 backfill만 재개하도록 분기를 추가했다. 이 게이팅을 `Expected.WorkItemID`가 채워진 일반 candidate-match 충돌 테스트들(`readyInput()` 기반)과 분리해, 기존 conflict/revision-mismatch 동작에 영향을 주지 않게 했다. +- `roadmapsync.OrderedSteps`/`storage.OrderedRoadmapSyncSteps`에 `identity_backfilled`를 `develop_matched` 바로 다음 순서로 추가했으므로, candidate-matched 일반 흐름(`runRemainingSteps`)도 이 step을 순회한다. 이 흐름에서는 develop scan이 이미 identity가 박힌 markdown을 매칭한 상태이므로 실제 backfill 액션이 불필요해, `runStep`에서 `StepIdentityBackfilled`를 `StepDevelopMatched`와 동일하게 no-op으로 처리하고 원장에는 마킹만 한다. 실제 git backfill 호출은 missing-create 최초/재시도 경로에서만 발생한다. +- `authoring/identity_write.go`의 identity block 렌더링을 `RenderProviderIdentityBlock` exported 함수로 추출했다(체크리스트 항목 "reuse identity block rendering without dirty-worktree assumptions"). 출력 포맷은 기존과 동일해 `identity_write_test.go`는 수정 없이 그대로 통과한다. +- Plan은 `queries/roadmap_sync_steps.sql`이나 sqlc 재생성 언급이 없었고, 이번 마이그레이션은 CHECK 제약만 갱신하므로 컬럼 스키마 변경이 없어 `db` 패키지 재생성이 필요 없음을 확인했다(코드 인스펙션 근거). + +## 주요 설계 결정 + +- Core-authored backfill 커밋 스코프 검증은 두 단계로 구성했다: (1) `git diff --cached --name-only`로 staged 파일이 대상 Milestone 경로 단 하나인지 확인, (2) `git diff --cached -- `의 모든 `+`/`-` 라인을 검사해 추가된 라인이 렌더링된 identity block과 정확히 일치하고 삭제된 라인이 전혀 없는지 확인한다. 두 조건 중 하나라도 위반되면 커밋하지 않고 에러를 반환한다. +- `GitBackfiller.BackfillIdentity`는 대상 파일에 이미 일치하는 identity가 있으면(이전 시도에서 push는 성공했지만 ledger mark가 실패한 재시도 케이스) 아무 것도 하지 않고 `nil`을 반환해 멱등성을 보장한다. 기존 identity가 있는데 값이 다르면 `ErrIdentityBackfillMismatch`를 반환해 다른 provider identity를 조용히 덮어쓰지 않는다. +- backfill 커밋 메시지에 `NomadCode-Identity-Backfill: true` marker 라인을 남겨, 이후 self-loop(03+01,02 subtask) 단계에서 재전달된 Gito webhook이 Core 자신의 backfill 커밋인지 식별할 수 있는 여지를 남겼다(이번 plan 범위는 아니며 marker만 기록). +- `Service.backfiller`가 `nil`인 상태에서 missing-create/backfill-resume 경로에 도달하면 명시적 에러를 반환한다(`identity backfiller not configured`). `GITO_DEVELOP_REPO_PATH`가 설정되지 않으면 `main.go`에서 아예 backfiller를 wiring하지 않으므로, 이 상황이 실제 misconfiguration 신호가 된다. +- `roadmap_sync_jobs.go`와 `scheduler.New` 시그니처는 변경하지 않았다. `Backfiller`는 `roadmapsyncpipeline.Service` 내부 의존성이며 River job args나 `CreationSyncRunner` 인터페이스에는 노출되지 않으므로, 스케줄러 wiring 코드는 그대로 두고 `main.go`의 구성 지점에서만 `SetBackfiller`를 호출했다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- Implementation waited for predecessor `01_scan_create_ledger` completion. +- `identity_backfilled` is durable and ordered after `develop_matched`. +- Backfill commit stages only the target Milestone identity block and records ledger only after push success. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### API-1 중간 검증 +```sh +$ cd services/core && go test -count=1 ./internal/roadmapsync ./internal/storage +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.003s +ok github.com/nomadcode/nomadcode-core/internal/storage 0.003s +``` + +### API-2 중간 검증 +```sh +$ cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/authoring +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 0.265s +ok github.com/nomadcode/nomadcode-core/internal/authoring 0.093s +``` + +### API-3 중간 검증 +```sh +$ cd services/core && go test -count=1 ./internal/scheduler ./cmd/server ./internal/roadmapsyncpipeline +ok github.com/nomadcode/nomadcode-core/internal/scheduler 2.037s +ok github.com/nomadcode/nomadcode-core/cmd/server 0.005s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 0.298s +``` + +### 최종 검증 +```sh +$ git diff --check +(no output, exit 0) + +$ cd services/core && go test -count=1 ./internal/roadmapsync ./internal/storage ./internal/authoring ./internal/roadmapsyncpipeline ./internal/scheduler ./cmd/server +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.004s +ok github.com/nomadcode/nomadcode-core/internal/storage 0.003s +ok github.com/nomadcode/nomadcode-core/internal/authoring 0.139s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 0.340s +ok github.com/nomadcode/nomadcode-core/internal/scheduler 2.033s +ok github.com/nomadcode/nomadcode-core/cmd/server 0.005s + +$ cd services/core && go vet ./internal/roadmapsync ./internal/storage ./internal/authoring ./internal/roadmapsyncpipeline ./internal/scheduler ./cmd/server +(no output, exit 0) +``` + +### 추가 회귀 확인 (plan 범위 밖, 안전을 위해 추가 실행) +```sh +$ cd services/core && go build ./... && go test -count=1 ./... +ok github.com/nomadcode/nomadcode-core/cmd/plane-smoke 0.004s +ok github.com/nomadcode/nomadcode-core/cmd/server 0.007s +ok github.com/nomadcode/nomadcode-core/internal/adapters/a2a 0.009s +ok github.com/nomadcode/nomadcode-core/internal/adapters/jira 0.012s +ok github.com/nomadcode/nomadcode-core/internal/adapters/mattermost 0.011s +ok github.com/nomadcode/nomadcode-core/internal/adapters/openai 0.013s +ok github.com/nomadcode/nomadcode-core/internal/adapters/plane 0.013s +ok github.com/nomadcode/nomadcode-core/internal/authoring 0.328s +ok github.com/nomadcode/nomadcode-core/internal/config 0.003s +ok github.com/nomadcode/nomadcode-core/internal/gitoevents 0.005s +ok github.com/nomadcode/nomadcode-core/internal/gitosync 1.405s +ok github.com/nomadcode/nomadcode-core/internal/http 0.010s +ok github.com/nomadcode/nomadcode-core/internal/notification 0.003s +ok github.com/nomadcode/nomadcode-core/internal/projectsync 0.005s +ok github.com/nomadcode/nomadcode-core/internal/protosocket 0.020s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.003s +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 0.354s +ok github.com/nomadcode/nomadcode-core/internal/scheduler 2.041s +ok github.com/nomadcode/nomadcode-core/internal/storage 0.004s +ok github.com/nomadcode/nomadcode-core/internal/workflow 0.004s +ok github.com/nomadcode/nomadcode-core/internal/workitem 0.003s +ok github.com/nomadcode/nomadcode-core/internal/workitempipeline 0.004s +``` + +### Git 커밋 스코프 증거 (temp repo) +```sh +$ go test -count=1 -run TestGitBackfiller ./internal/roadmapsyncpipeline -v +=== RUN TestGitBackfillerCommitsAndPushesIdentityOnly +--- PASS: TestGitBackfillerCommitsAndPushesIdentityOnly (0.13s) +=== RUN TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted +--- PASS: TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted (0.10s) +=== RUN TestGitBackfillerRejectsMismatchedExistingIdentity +--- PASS: TestGitBackfillerRejectsMismatchedExistingIdentity (0.08s) +PASS +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 0.312s +``` +`TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted`는 backfill 대상 파일에 identity block과 무관한 dirty edit이 섞여 있으면 scope check가 커밋 자체를 거부하고 `origin/develop`이 최초 커밋에 그대로 머무는지 확인한다(unrelated roadmap 변경이 포함되지 않는다는 요구사항의 직접 증거). + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** + +## 코드리뷰 결과 + +- 종합 판정: FAIL +- 차원별 평가: + - Correctness: Fail + - Completeness: Fail + - Test coverage: Fail + - API contract: Pass + - Code quality: Pass + - Implementation deviation: Fail + - Verification trust: Warn + - Spec conformance: Fail +- 발견된 문제: + - Required: `services/core/internal/roadmapsyncpipeline/git_backfiller.go:73`에서 worktree에 존재하는 matching identity block만 보고 `nil`을 반환한다. 하지만 같은 함수가 `verifyScopedDiff` 실패 또는 `git push` 실패 후에도 worktree/local commit에 identity block을 남길 수 있어, 다음 retry가 remote `develop` push 성공 없이 `identity_backfilled` ledger를 mark할 수 있다. `BackfillIdentity`는 remote target branch 또는 성공적으로 pushed commit에 matching identity가 있음을 확인한 경우에만 no-op으로 성공해야 하며, pre-commit 실패 시 staged/worktree 변경을 원상복구하거나 retry가 false success로 흐르지 않게 해야 한다. push 실패 후 retry는 pending commit을 다시 push하거나 remote 미반영 상태를 에러로 유지해야 한다. + - Required: `services/core/internal/roadmapsyncpipeline/git_backfiller.go:66`가 verified `origin/develop` revision이 아니라 현재 working tree/`HEAD`의 `RepoPath/path`를 읽어 backfill commit을 만든다. `BranchRevisionScanner`는 `services/core/internal/gitosync/scanner.go:124`와 `services/core/internal/gitosync/scanner.go:174`에서 remote ref를 fetch/`git show`로 검증할 뿐 local branch checkout을 업데이트하지 않으므로, 정상 missing-create 이벤트에서도 새 Milestone 파일이 worktree에 없거나 stale `HEAD` 위에서 `HEAD:develop` push가 시도될 수 있다. Backfiller에 verified roadmap revision을 전달하고, 쓰기 전에 local branch/working tree가 그 revision을 기준으로 깨끗한지 보장하거나, remote revision에서 별도 worktree를 만들어 identity-only commit을 생성하도록 고쳐야 한다. +- 다음 단계: FAIL follow-up PLAN/CODE_REVIEW를 같은 task directory에 작성한다. `USER_REVIEW.md`는 생성하지 않는다. diff --git a/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_1.log b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_1.log new file mode 100644 index 0000000..d04a5f7 --- /dev/null +++ b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_1.log @@ -0,0 +1,248 @@ + + +# Code Review Reference - REVIEW_API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service blockers, generic scope changes, repeated failures, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only Milestone lock decisions in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-07-02 +task=m-agent-origin-milestone-creation-sync/02+01_identity_backfill, plan=1, tag=REVIEW_API + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/agent-origin-milestone-creation-sync.md` +- Task ids: + - `identity-backfill`: Plane 생성 직후 Milestone markdown에 `Provider identity` block을 주입하고 Core-authored backfill commit을 `develop`에 push한다. +- Completion mode: check-on-pass + +## Archive Evidence Snapshot + +- Prior logs: + - `agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_0.log` + - `agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_0.log` +- Verdict: FAIL +- Required summary: + - `services/core/internal/roadmapsyncpipeline/git_backfiller.go:73` treats a matching identity in local worktree as success, so retry after scope-check or push failure can mark `identity_backfilled` without a successful remote push. + - `services/core/internal/roadmapsyncpipeline/git_backfiller.go:66` reads current working tree/`HEAD`, while `services/core/internal/gitosync/scanner.go:124` and `services/core/internal/gitosync/scanner.go:174` verify remote `origin/develop` without updating local checkout. +- Suggested/Nit summary: 없음. +- Affected files: `services/core/internal/roadmapsyncpipeline/git_backfiller.go`, `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go`, `services/core/internal/roadmapsyncpipeline/service.go`, `services/core/internal/roadmapsyncpipeline/service_test.go`, optional `services/core/cmd/server/main.go`. +- Verification evidence from failed loop: `git diff --check`, focused `go test -count=1`, and focused `go vet` passed, but tests did not cover retry false-success or stale checkout base. +- Allowed prior-context reread: only the two prior log files listed above if exact review wording is needed. + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-cloud-G08.md` -> `code_review_cloud_G08_N.log`, `PLAN-cloud-G08.md` -> `plan_cloud_G08_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. `USER_REVIEW.md`가 연결된 Milestone 결정으로 완료/PASS 해소되면 code-review가 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log` 작성 후 archive 이동한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_API-1] Remote Push Success And Retry Contract | [x] | +| [REVIEW_API-2] Verified Develop Revision Base | [x] | + +## 구현 체크리스트 + +- [x] `GitBackfiller`가 local worktree/local commit identity만으로 성공하지 않고, remote target branch 또는 성공적으로 pushed commit에 matching identity가 확인될 때만 no-op 성공하도록 수정한다. +- [x] Backfill commit이 scanner가 검증한 develop revision을 기준으로 생성되도록 `BackfillInput`/service 경로에 revision을 전달하고, dirty/stale checkout은 파일 mutation 전에 명시적으로 실패시키거나 안전한 임시 worktree로 처리한다. +- [x] scope-check 실패 retry, push 실패 retry, local HEAD behind `origin/develop`, mismatched milestone id identity block에 대한 regression tests를 추가한다. +- [x] `git diff --check`, focused `go test -count=1`, focused `go vet`를 실행한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G08_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_cloud_G08_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [x] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [x] PASS이면 active task 디렉터리 `agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/`를 `agent-task/archive/YYYY/MM/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [x] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [x] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-agent-origin-milestone-creation-sync/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-cloud-G08.md`와 `CODE_REVIEW-cloud-G08.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가 연결된 Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +- `cmd/server/main.go`의 backfiller wiring은 기존 구현 범위의 변경으로 유지했다. 이번 follow-up은 `NewGitBackfiller` 시그니처를 바꾸지 않고 verified revision을 `BackfillInput.RoadmapRevision`으로 전달하는 방식이라, 해당 wiring을 추가 수정하지 않았다. +- REVIEW_API-2의 두 옵션(임시 worktree vs. verified remote ref로 clean checkout/reset) 중 후자를 택했다. 기존 `TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted`가 dirty checkout을 명시적 실패로 기대하므로, primary develop checkout에서 dirty guard 후 `git checkout -B `로 verified revision에 재기준하는 방식이 기존 계약과 일치한다. +- 검증 명령은 계획의 고정 계약 그대로 실행했다. 대체 없음. + +## 주요 설계 결정 + +- 성공/mismatch 판정을 전부 **remote develop 상태 기준**으로 옮겼다. `BackfillIdentity`는 먼저 `git fetch --prune`으로 remote branch를 당긴 뒤 `origin/develop`의 revision을 `git show :`로 읽고, 그 내용에서 identity를 파싱한다. matching이면 no-op 성공, 다른 identity면 `ErrIdentityBackfillMismatch`, 없으면 backfill을 진행한다. 이로써 local worktree/local commit만 identity를 가진 상태는 절대 성공으로 취급되지 않는다 (REVIEW_API-1). +- Commit base를 verified remote revision으로 고정했다. dirty guard(`git status --porcelain`) 통과 후 `git checkout -B `로 로컬 브랜치를 remote tip에 재기준하고, 그 위에 identity-only 변경을 commit한다. 로컬 HEAD가 `origin/develop`보다 뒤처져 있어도 commit parent는 항상 verified revision이다 (REVIEW_API-2). +- `BackfillInput`에 `RoadmapRevision`을 추가하고 service의 두 backfill 경로(fresh missing-create, redelivered resume)에서 `in.Scan.Revision.Normalize().Revision`을 전달한다. backfiller는 이 값이 remote tip과 다르면 파일 mutation 전에 실패시켜, scanner가 검증하지 않은 revision 위에 commit하지 않는다. +- pre-push 실패(write/add/scope-check/commit)는 `cleanup`으로 `git reset --hard `을 수행해 staged/worktree 잔여물을 제거하고 원인 에러를 보존한다. push 실패는 로컬 commit을 남기되 에러를 반환하며, 다음 retry가 remote revision으로 재기준(`checkout -B`)해 재생성/재push한다. ledger step은 호출자(`backfillIdentityAndMark`)가 nil 반환 후에만 mark하므로 실패한 push는 ledger를 전진시키지 않는다. +- `identityMatches`에 canonical milestone id(`MilestoneID()`) 비교를 추가해, 같은 provider/tenant/project/work item id를 다른 milestone id로 재사용하는 블록을 mismatch로 판정한다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- Retry after scope-check failure or push failure cannot mark `identity_backfilled` until remote develop contains the identity commit. +- Backfill commit is based on the verified develop revision observed by the scanner, not stale local `HEAD`. +- Existing identity mismatch includes canonical milestone id, not only provider/work item id. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +필수 규칙: +- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. +- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. +- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. +- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. +- mobile/UI hang, timeout, 또는 2분 무진행은 blind retry를 중단하고 focused rerun 명령과 screenshot/window/UI-tree evidence path를 남기며, 불가능하면 정확한 사유를 남긴다. + +### REVIEW_API-1 중간 검증 +```sh +$ cd services/core && go test -count=1 -run 'TestGitBackfiller' ./internal/roadmapsyncpipeline -v +=== RUN TestGitBackfillerCommitsAndPushesIdentityOnly +--- PASS: TestGitBackfillerCommitsAndPushesIdentityOnly (0.26s) +=== RUN TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted +--- PASS: TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted (0.11s) +=== RUN TestGitBackfillerRejectsMismatchedExistingIdentity +--- PASS: TestGitBackfillerRejectsMismatchedExistingIdentity (0.16s) +=== RUN TestGitBackfillerRejectsMismatchedMilestoneID +--- PASS: TestGitBackfillerRejectsMismatchedMilestoneID (0.16s) +=== RUN TestGitBackfillerRetriesAfterPushFailure +--- PASS: TestGitBackfillerRetriesAfterPushFailure (0.26s) +=== RUN TestGitBackfillerCommitsOnVerifiedRemoteRevisionWhenLocalBehind +--- PASS: TestGitBackfillerCommitsOnVerifiedRemoteRevisionWhenLocalBehind (0.19s) +=== RUN TestGitBackfillerRejectsMovedRemoteRevision +--- PASS: TestGitBackfillerRejectsMovedRemoteRevision (0.15s) +PASS +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 1.287s +``` + +### REVIEW_API-2 중간 검증 +```sh +$ cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/gitosync +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 2.037s +ok github.com/nomadcode/nomadcode-core/internal/gitosync 1.409s +``` + +### 최종 검증 +```sh +$ git diff --check +(exit 0, no output) + +$ cd services/core && go test -count=1 -run 'TestGitBackfiller' ./internal/roadmapsyncpipeline -v +=== RUN TestGitBackfillerCommitsAndPushesIdentityOnly +--- PASS: TestGitBackfillerCommitsAndPushesIdentityOnly (0.28s) +=== RUN TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted +--- PASS: TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted (0.12s) +=== RUN TestGitBackfillerRejectsMismatchedExistingIdentity +--- PASS: TestGitBackfillerRejectsMismatchedExistingIdentity (0.16s) +=== RUN TestGitBackfillerRejectsMismatchedMilestoneID +--- PASS: TestGitBackfillerRejectsMismatchedMilestoneID (0.18s) +=== RUN TestGitBackfillerRetriesAfterPushFailure +--- PASS: TestGitBackfillerRetriesAfterPushFailure (0.32s) +=== RUN TestGitBackfillerCommitsOnVerifiedRemoteRevisionWhenLocalBehind +--- PASS: TestGitBackfillerCommitsOnVerifiedRemoteRevisionWhenLocalBehind (0.36s) +=== RUN TestGitBackfillerRejectsMovedRemoteRevision +--- PASS: TestGitBackfillerRejectsMovedRemoteRevision (0.25s) +PASS +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 1.672s + +$ cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server +ok github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline 2.352s +ok github.com/nomadcode/nomadcode-core/internal/gitosync 1.421s +ok github.com/nomadcode/nomadcode-core/cmd/server 0.010s + +$ cd services/core && go vet ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server +(exit 0, no output) +``` + +보조 확인(계획의 테스트 환경 규칙에 명시된 관련 패키지, 계약 명령 아님): +```sh +$ cd services/core && go test -count=1 ./internal/roadmapsync ./internal/storage ./internal/authoring ./internal/scheduler +ok github.com/nomadcode/nomadcode-core/internal/roadmapsync 0.006s +ok github.com/nomadcode/nomadcode-core/internal/storage 0.007s +ok github.com/nomadcode/nomadcode-core/internal/authoring 0.208s +ok github.com/nomadcode/nomadcode-core/internal/scheduler 2.040s +``` + +--- + +> **[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. + +Sections and their ownership: + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, 개요, 리뷰 에이전트 지시 | Fixed at stub creation | Implementing agent must not modify or execute these | +| Roadmap Targets | Fixed at stub creation from plan | Implementing agent must not modify; code-review copies it into `complete.log` as `Roadmap Completion` only on PASS | +| Archive Evidence Snapshot | Fixed at stub creation from plan | Implementing agent uses it as default prior-loop context; read only the specific archive files cited there when more detail is required | +| 구현 항목별 완료 여부 (item names) | Fixed at stub creation | Implementing agent checks `[ ]` -> `[x]` only | +| 구현 체크리스트 (item text/order) | Fixed at stub creation from plan | Implementing agent checks `[ ]` -> `[x]` only; final checkbox is mandatory before saving | +| 코드리뷰 전용 체크리스트 | Review agent only | Implementing agent must not modify or check this section | +| 계획 대비 변경 사항, 주요 설계 결정 | Implementing agent | Replace placeholder text with actual content | +| 사용자 리뷰 요청 | Implementing agent | Keep `상태: 없음` unless a selected Milestone `구현 잠금 > 결정 필요` item blocks implementation | +| 리뷰어를 위한 체크포인트 | Fixed at stub creation | Pre-filled from plan | +| 검증 결과 (section headings + commands) | Fixed at stub creation | Implementing agent fills in command output only; command changes require a `계획 대비 변경 사항` entry | +| 코드리뷰 결과 | Review agent appends | Not included in stub | + +## 코드리뷰 결과 + +- 종합 판정: PASS +- 차원별 평가: + - correctness: Pass + - completeness: Pass + - test coverage: Pass + - API contract: Pass + - code quality: Pass + - implementation deviation: Pass + - verification trust: Pass + - spec conformance: Pass +- 발견된 문제: 없음 +- 다음 단계: PASS이므로 `complete.log`를 작성하고 active task directory를 `agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/`로 이동한다. `m-agent-origin-milestone-creation-sync` 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 호출은 수행하지 않는다. + +리뷰 메모: +- SDD S04(`identity-backfill`) Evidence Map과 일치한다. `GitBackfiller`는 remote `origin/develop` 상태를 fetch/read하고, scan-verified revision이 움직였으면 mutation 전에 실패한다. +- retry false-success 경로가 닫혀 있다. scope-check/dirty checkout은 성공으로 전환되지 않고, push 실패 후 retry도 remote에 identity가 실제 반영된 뒤에만 nil을 반환한다. +- `identity_backfilled` ledger step은 storage/domain enum, migration, service orchestration, regression tests에 반영됐다. +- 재검증: + - `git diff --check` PASS + - `cd services/core && go test -count=1 -run 'TestGitBackfiller' ./internal/roadmapsyncpipeline -v` PASS + - `cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server` PASS + - `cd services/core && go vet ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server` PASS + - `cd services/core && go test ./...` PASS + - `cd services/core && go vet ./...` PASS diff --git a/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/complete.log b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/complete.log new file mode 100644 index 0000000..1e3d07c --- /dev/null +++ b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/complete.log @@ -0,0 +1,49 @@ +# Complete - m-agent-origin-milestone-creation-sync/02+01_identity_backfill + +## 완료 일시 + +2026-07-02 + +## 요약 + +Agent-origin Milestone creation sync의 `identity-backfill` task를 2회 리뷰 루프로 완료했다. 최종 판정은 PASS다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_cloud_G08_0.log` | `code_review_cloud_G08_0.log` | FAIL | local-only identity success, stale checkout base 문제가 남아 follow-up 필요 | +| `plan_cloud_G08_1.log` | `code_review_cloud_G08_1.log` | PASS | remote push success predicate, verified revision base, retry regression tests 충족 | + +## 구현/정리 내용 + +- `identity_backfilled` ledger step을 storage/domain ordered step set, DB check constraint migration, service orchestration에 추가했다. +- `GitBackfiller`를 추가해 scanner가 검증한 remote develop revision 위에서 provider identity block만 commit/push하도록 했다. +- retry false-success를 막기 위해 already-satisfied/mismatch 판정을 local worktree가 아니라 remote target branch 기준으로 수행한다. +- `BackfillInput.RoadmapRevision`을 service/backfiller 경로에 전달해 stale remote revision은 mutation 전에 실패시킨다. +- identity block rendering을 `authoring.RenderProviderIdentityBlock`으로 공유해 authoring writer와 Core backfill writer가 같은 Markdown shape를 사용한다. +- scope-check failure, push failure retry, local HEAD behind `origin/develop`, moved remote revision, mismatched milestone id regression tests를 추가했다. + +## 최종 검증 + +- `git diff --check` - PASS; output 없음 +- `cd services/core && go test -count=1 -run 'TestGitBackfiller' ./internal/roadmapsyncpipeline -v` - PASS; GitBackfiller regression tests 통과 +- `cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server` - PASS +- `cd services/core && go vet ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server` - PASS; output 없음 +- `cd services/core && go test ./...` - PASS +- `cd services/core && go vet ./...` - PASS; output 없음 + +## Roadmap Completion + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/agent-origin-milestone-creation-sync.md` +- Completed task ids: + - `identity-backfill`: PASS; evidence=`agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_1.log`, `agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_1.log`; verification=`git diff --check`, `cd services/core && go test -count=1 -run 'TestGitBackfiller' ./internal/roadmapsyncpipeline -v`, `cd services/core && go test ./...` +- Not completed task ids: 없음 + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/PLAN-cloud-G08.md b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_0.log similarity index 100% rename from agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/PLAN-cloud-G08.md rename to agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_0.log diff --git a/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_1.log b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_1.log new file mode 100644 index 0000000..85b6909 --- /dev/null +++ b/agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_1.log @@ -0,0 +1,265 @@ + + +# Plan - REVIEW_API + +## 이 파일을 읽는 구현 에이전트에게 + +구현 후 active `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 반드시 실제 구현 내용과 검증 출력으로 채운다. 검증을 실행하고, active 파일은 그대로 둔 뒤 리뷰 준비를 보고한다. 최종 판정, log rename, `complete.log`, archive 이동은 code-review 스킬 전용이다. + +선택된 Milestone의 `구현 잠금 > 결정 필요` 항목이 실구현을 막을 때만 review stub의 `사용자 리뷰 요청` 섹션을 채우고 멈춘다. 구현 중 사용자에게 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 환경/secret/service 준비, 일반 범위 조정, 검증 증거 공백은 사용자 리뷰 요청이 아니다. + +## 배경 + +이 follow-up은 `plan_cloud_G08_0.log` / `code_review_cloud_G08_0.log`의 FAIL 판정을 수습한다. 기존 구현은 identity block이 local worktree나 local commit에만 남아 있어도 backfill 성공으로 간주할 수 있고, scanner가 검증한 `origin/develop` revision이 아니라 현재 checkout `HEAD` 위에서 backfill commit을 만들 수 있다. S04는 "identity-only commit이 develop에 push된 뒤에만 ledger step을 mark"해야 하므로, git retry와 revision anchoring을 다시 닫는다. + +## 사용자 리뷰 요청 흐름 + +사용자 리뷰 요청은 선택된 Milestone lock decision만 active review stub의 `사용자 리뷰 요청` 섹션에 기록한다. 직접 사용자 prompt는 금지이며, code-review가 정당성을 검증하고 실제 `USER_REVIEW.md` 작성을 소유한다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/agent-origin-milestone-creation-sync.md` +- Task ids: + - `identity-backfill`: Plane 생성 직후 Milestone markdown에 `Provider identity` block을 주입하고 Core-authored backfill commit을 `develop`에 push한다. +- Completion mode: check-on-pass + +## Archive Evidence Snapshot + +- Prior logs: + - `agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_0.log` + - `agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_0.log` +- Verdict: FAIL +- Required summary: + - `services/core/internal/roadmapsyncpipeline/git_backfiller.go:73` treats a matching identity in local worktree as success, so retry after scope-check or push failure can mark `identity_backfilled` without a successful remote push. + - `services/core/internal/roadmapsyncpipeline/git_backfiller.go:66` reads current working tree/`HEAD`, while `services/core/internal/gitosync/scanner.go:124` and `services/core/internal/gitosync/scanner.go:174` verify remote `origin/develop` without updating local checkout. +- Suggested/Nit summary: 없음. +- Affected files: `services/core/internal/roadmapsyncpipeline/git_backfiller.go`, `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go`, `services/core/internal/roadmapsyncpipeline/service.go`, `services/core/internal/roadmapsyncpipeline/service_test.go`, optional `services/core/cmd/server/main.go`. +- Verification evidence from failed loop: `git diff --check`, focused `go test -count=1`, and focused `go vet` passed, but tests did not cover retry false-success or stale checkout base. +- Allowed prior-context reread: only the two prior log files listed above if exact review wording is needed. + +## 분석 결과 + +### 읽은 파일 + +- `agent-ops/rules/project/rules.md` +- `agent-ops/rules/private/rules.md` +- `agent-ops/rules/common/rules-roadmap.md` +- `agent-ops/skills/common/router.md` +- `agent-ops/skills/common/code-review/SKILL.md` +- `agent-ops/skills/common/plan/SKILL.md` +- `agent-ops/skills/common/_templates/implementation-user-review-request-section.md` +- `agent-ops/rules/project/domain/core/rules.md` +- `agent-ops/rules/project/domain/workspace-ops/rules.md` +- `agent-test/local/rules.md` +- `agent-test/local/core-smoke.md` +- `agent-contract/index.md` +- `agent-contract/outer/gito-branch-webhook-consumer-v1.md` +- `agent-roadmap/current.md` +- `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/agent-origin-milestone-creation-sync.md` +- `agent-roadmap/sdd/agent-ops-mcp-control-plane/agent-origin-milestone-creation-sync/SDD.md` +- `agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/01_scan_create_ledger/complete.log` +- `agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/plan_cloud_G08_0.log` +- `agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/code_review_cloud_G08_0.log` +- `services/core/internal/roadmapsyncpipeline/service.go` +- `services/core/internal/roadmapsyncpipeline/service_test.go` +- `services/core/internal/roadmapsyncpipeline/git_backfiller.go` +- `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go` +- `services/core/internal/authoring/identity_write.go` +- `services/core/internal/roadmapsync/identity.go` +- `services/core/internal/roadmapsync/milestone_parser.go` +- `services/core/internal/roadmapsync/develop_match.go` +- `services/core/internal/roadmapsync/retry.go` +- `services/core/internal/roadmapsync/retry_test.go` +- `services/core/internal/storage/roadmap_sync_steps.go` +- `services/core/internal/storage/roadmap_sync_steps_test.go` +- `services/core/internal/gitosync/scanner.go` +- `services/core/internal/gitosync/bridge.go` +- `services/core/internal/gitosync/bridge_test.go` +- `services/core/cmd/server/main.go` +- `services/core/cmd/server/main_test.go` +- `services/core/migrations/00007_create_roadmap_sync_steps.sql` +- `services/core/migrations/00010_add_roadmap_identity_backfilled_step.sql` +- `services/core/queries/roadmap_sync_steps.sql` +- `services/core/internal/config/config.go` + +### SDD 기준 + +- SDD: `agent-roadmap/sdd/agent-ops-mcp-control-plane/agent-origin-milestone-creation-sync/SDD.md` +- 상태: `[승인됨]`, SDD 잠금 해제, 사용자 리뷰 없음. +- Targeted scenario: S04=`identity-backfill`. +- Evidence Map row: identity backfill writer test and git commit scope check. +- 이 follow-up은 S04의 "identity-only backfill commit이 develop에 push되고 unrelated 변경은 포함되지 않는다"를 remote push 성공, retry idempotency, verified revision base까지 확장해 검증한다. + +### 테스트 환경 규칙 + +- test_env: `local`. +- `agent-test/local/rules.md`와 `agent-test/local/core-smoke.md`를 읽었다. +- 적용 명령: current checkout focused Go tests plus `git diff --check`; core-smoke baseline은 `services/core`에서 `go test ./...`, 보조로 `go vet ./...`를 요구한다. +- 현재 리뷰 재검증: `git diff --check`, `go test -count=1 ./internal/roadmapsync ./internal/storage ./internal/authoring ./internal/roadmapsyncpipeline ./internal/scheduler ./cmd/server`, `go vet ./internal/roadmapsync ./internal/storage ./internal/authoring ./internal/roadmapsyncpipeline ./internal/scheduler ./cmd/server` 모두 exit 0. +- 이번 follow-up은 git temp repo 단위 테스트로 재현 가능하므로 remote service/secret preflight는 필요 없다. + +### 테스트 커버리지 공백 + +- `TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted`는 첫 실패 후 remote가 안 변한 것만 확인하고, 두 번째 `BackfillIdentity` 호출이 false success가 되지 않는지 확인하지 않는다. +- push 실패 후 retry가 remote push 성공 전 `nil`을 반환하지 않는 테스트가 없다. +- local `HEAD`가 `origin/develop`보다 뒤처진 checkout에서 backfill commit이 verified revision을 기준으로 만들어지는지 확인하는 테스트가 없다. +- existing identity block이 같은 provider/work item이지만 다른 milestone id일 때 mismatch로 거부하는 테스트가 없다. + +### 심볼 참조 + +- 기존 변경에서 새 symbol `BackfillInput`, `Backfiller`, `GitBackfiller`, `SetBackfiller`, `StepIdentityBackfilled`가 추가됐다. +- 이번 follow-up에서 `BackfillInput` 필드를 늘리거나 `GitBackfiller` helper를 추가하면 `rg --sort path "BackfillInput|BackfillIdentity|SetBackfiller|StepIdentityBackfilled" services/core`로 모든 call site를 갱신한다. + +### 분할 판단 + +- Split decision policy evaluated before writing this follow-up. +- 기존 subtask는 `02+01_identity_backfill`이며 predecessor `01`은 `agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/01_scan_create_ledger/complete.log`로 PASS 확인됐다. +- 이 follow-up은 같은 subtask 내부의 FAIL 수습이며, 두 Required가 같은 `GitBackfiller`/pipeline seam에 묶여 있어 별도 split은 오히려 retry contract를 중복 수정하게 만든다. + +### 범위 결정 근거 + +- 포함: S04 backfill commit base, retry success criteria, ledger mark condition, git temp repo regression tests. +- 제외: self-loop webhook classification, Plane body/title update projection, field smoke, provider native API schema 일반화. + +### 빌드 등급 + +- `cloud-G08`: git process/push semantics, persisted ledger step, remote revision anchoring, retry false-success 진단이 모두 필요하다. + +## 의존 관계 및 구현 순서 + +`02+01_identity_backfill` depends on sibling predecessor `01_scan_create_ledger`; predecessor PASS is satisfied by `agent-task/archive/2026/07/m-agent-origin-milestone-creation-sync/01_scan_create_ledger/complete.log`. + +## 구현 체크리스트 + +- [ ] `GitBackfiller`가 local worktree/local commit identity만으로 성공하지 않고, remote target branch 또는 성공적으로 pushed commit에 matching identity가 확인될 때만 no-op 성공하도록 수정한다. +- [ ] Backfill commit이 scanner가 검증한 develop revision을 기준으로 생성되도록 `BackfillInput`/service 경로에 revision을 전달하고, dirty/stale checkout은 파일 mutation 전에 명시적으로 실패시키거나 안전한 임시 worktree로 처리한다. +- [ ] scope-check 실패 retry, push 실패 retry, local HEAD behind `origin/develop`, mismatched milestone id identity block에 대한 regression tests를 추가한다. +- [ ] `git diff --check`, focused `go test -count=1`, focused `go vet`를 실행한다. +- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## [REVIEW_API-1] Remote Push Success And Retry Contract + +### 문제 + +`services/core/internal/roadmapsyncpipeline/git_backfiller.go:73`은 target file에 matching identity block이 있으면 즉시 `nil`을 반환한다. 같은 함수의 `services/core/internal/roadmapsyncpipeline/git_backfiller.go:86` 이후 경로는 scope-check 실패나 push 실패 뒤 worktree/local commit에 identity block을 남길 수 있어, 다음 retry가 remote push 성공 없이 `identity_backfilled` ledger를 mark할 수 있다. + +### 해결 방법 + +Remote target branch에서 matching identity가 확인된 경우에만 "already satisfied"로 성공한다. Pre-commit failure는 staged/worktree 변경을 cleanup하거나, cleanup 실패까지 에러에 포함해 retry가 false success가 되지 않게 한다. Push failure 뒤에는 remote 미반영 상태를 성공으로 해석하지 말고, 재시도 시 pending commit을 push하거나 remote 기준으로 안전하게 재생성한다. + +Before: + +```go +// services/core/internal/roadmapsyncpipeline/git_backfiller.go:73 +existing, parseErr := roadmapsync.ParseMilestoneIdentity(path, content) +if parseErr == nil { + if identityMatches(existing, in.Identity) { + return nil + } + return ErrIdentityBackfillMismatch +} +``` + +After: + +```go +// sketch; exact helper names are implementation-owned +if remoteHasMatchingIdentity(ctx, path, in.Identity, expectedRevision) { + return nil +} +if localHasIdentityButRemoteDoesNot(path, in.Identity) { + return errors.New("roadmapsyncpipeline: identity backfill is local-only and not pushed") +} +``` + +### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller.go`: success predicate uses remote/pushed state, not only local file content. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller.go`: failed scope-check and failed push paths do not leave a future retry able to return nil before remote confirmation. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller.go`: identity match compares provider, tenant, project, work item id, and canonical milestone id. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go`: second call after scope-check failure remains failing until dirty state is resolved and never advances remote. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go`: push failure followed by retry does not return success unless remote branch contains the identity commit. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go`: existing identity with a different milestone id returns `ErrIdentityBackfillMismatch`. + +### 테스트 작성 + +Regression tests required. Use temp bare/working repos; do not depend on live remotes or secrets. + +### 중간 검증 + +```sh +cd services/core && go test -count=1 -run 'TestGitBackfiller' ./internal/roadmapsyncpipeline -v +``` + +Expected: PASS, including tests for retry false-success and push failure. + +## [REVIEW_API-2] Verified Develop Revision Base + +### 문제 + +`services/core/internal/gitosync/scanner.go:124` fetches and verifies `refs/remotes//`, and `services/core/internal/gitosync/scanner.go:174` reads milestone markdown with `git show :`. It does not update the local working tree. `services/core/internal/roadmapsyncpipeline/git_backfiller.go:66` then reads `filepath.Join(RepoPath, path)` and commits `HEAD:`, so the backfill can be based on a stale checkout instead of the verified `after` revision. + +### 해결 방법 + +Pass the verified roadmap revision from `SyncCreationInput.Scan.Revision.Normalize().Revision` into the backfill path. Before writing, fetch/verify the remote ref still equals that revision and create the commit from that revision: either check out/reset a clean dedicated branch to the remote ref, or use a temporary worktree rooted at the verified commit. If the checkout is dirty or the remote ref has moved, fail before mutating files so the worker can retry from a fresh event. + +Before: + +```go +// services/core/internal/roadmapsyncpipeline/git_backfiller.go:66 +fullPath := filepath.Join(b.RepoPath, path) +contentBytes, err := os.ReadFile(fullPath) +``` + +After: + +```go +// sketch; exact helper names are implementation-owned +if err := b.prepareVerifiedDevelopBase(ctx, in.RoadmapRevision); err != nil { + return err +} +fullPath := filepath.Join(b.RepoPath, path) +``` + +### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/roadmapsyncpipeline/service.go`: pass the verified scan revision into `BackfillInput` for initial missing-create and resume paths. +- [ ] `services/core/internal/roadmapsyncpipeline/service_test.go`: fake backfiller asserts `BackfillInput` carries the scan revision. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller.go`: verify/fetch remote branch and base the commit on the verified revision before writing. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go`: local branch behind `origin/develop` still produces a backfill commit on top of `origin/develop`. +- [ ] `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go`: remote ref mismatch or dirty checkout fails before file mutation. + +### 테스트 작성 + +Regression tests required because this is the SDD S04 source-of-truth guarantee. + +### 중간 검증 + +```sh +cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/gitosync +``` + +Expected: PASS; temp repo test proves commit base follows verified `origin/develop`. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `services/core/internal/roadmapsyncpipeline/git_backfiller.go` | REVIEW_API-1, REVIEW_API-2 | +| `services/core/internal/roadmapsyncpipeline/git_backfiller_test.go` | REVIEW_API-1, REVIEW_API-2 | +| `services/core/internal/roadmapsyncpipeline/service.go` | REVIEW_API-2 | +| `services/core/internal/roadmapsyncpipeline/service_test.go` | REVIEW_API-2 | +| `services/core/cmd/server/main.go` | REVIEW_API-2, only if constructor wiring must pass new config into `GitBackfiller` | + +## 최종 검증 + +```sh +git diff --check +cd services/core && go test -count=1 -run 'TestGitBackfiller' ./internal/roadmapsyncpipeline -v +cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server +cd services/core && go vet ./internal/roadmapsyncpipeline ./internal/gitosync ./cmd/server +``` + +Expected: all commands exit 0 with fresh `-count=1` test execution. + +모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/CODE_REVIEW-cloud-G08.md b/agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/CODE_REVIEW-cloud-G08.md deleted file mode 100644 index 4e7f99e..0000000 --- a/agent-task/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/CODE_REVIEW-cloud-G08.md +++ /dev/null @@ -1,127 +0,0 @@ - - -# Code Review Reference - API - -> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** -> The task is NOT complete until every implementation-owned section below is filled in. -> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. -> Fill implementation-owned sections, then stop with active files in place and report ready for review. -> If implementation is blocked by a selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked 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. -> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only. - -## 개요 - -date=2026-07-02 -task=m-agent-origin-milestone-creation-sync/02+01_identity_backfill, plan=0, tag=API - -## Roadmap Targets - -- Milestone: `agent-roadmap/phase/agent-ops-mcp-control-plane/milestones/agent-origin-milestone-creation-sync.md` -- Task ids: - - `identity-backfill`: Plane 생성 직후 Milestone markdown에 `Provider identity` block을 주입하고 Core-authored backfill commit을 `develop`에 push한다. -- Completion mode: check-on-pass - -## 이 파일을 읽는 리뷰 에이전트에게 - -> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. - -각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. -리뷰 완료는 판정 append, active 파일 log archive, PASS 시 `complete.log` 작성과 task directory archive까지 끝난 상태를 의미합니다. - ---- - -## 구현 항목별 완료 여부 - -| 항목 | 완료 여부 | -|------|---------| -| [API-1] Durable Backfill Step | [ ] | -| [API-2] Identity Backfill Writer And Commit Scope | [ ] | -| [API-3] Scheduler Wiring And Review Evidence | [ ] | - -## 구현 체크리스트 - -- [ ] `identity-backfill` step을 storage/domain ordered step set에 추가하고 DB check constraint migration/query tests를 갱신한다. -- [ ] missing-create Plane create + identity upsert 직후 Milestone markdown에 `Provider identity` block을 주입하는 pipeline seam을 추가한다. -- [ ] backfill writer가 identity block 외 unrelated roadmap 변경을 commit하지 않는 git scope check를 구현/테스트한다. 검증: backfill commit에는 identity block 외의 unrelated roadmap 변경이 포함되지 않는다. -- [ ] push 성공 뒤에만 `identity_backfilled` ledger step을 mark하고, 실패 시 retry가 create를 반복하지 않고 backfill부터 재개하게 한다. -- [ ] `git diff --check`, focused `go test -count=1`, focused `go vet`를 실행한다. -- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. - -## 코드리뷰 전용 체크리스트 - -> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. - -- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. -- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. -- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_cloud_G08_N.log`로 아카이브한다. -- [ ] active `PLAN-*-G??.md`를 `plan_cloud_G08_M.log`로 아카이브한다. -- [ ] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. -- [ ] PASS이면 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. -- [ ] PASS이면 active task 디렉터리를 `agent-task/archive/YYYY/MM/m-agent-origin-milestone-creation-sync/02+01_identity_backfill/`로 이동한다. -- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. -- [ ] PASS split 작업이면 이동 후 빈 active parent를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. -- [ ] WARN/FAIL이면 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성하고 `complete.log`를 작성하지 않는다. - -## 계획 대비 변경 사항 - -_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ - -## 주요 설계 결정 - -_구현 에이전트가 주요 설계 결정 사항을 기록한다._ - -## 사용자 리뷰 요청 - -_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ - -- 상태: 없음 -- 사유 유형: 없음 -- 연결 대상: 없음 -- 결정 필요: 없음 -- 차단 근거: 없음 -- 실행한 검증/명령: 없음 -- 자동 후속 불가 이유: 없음 -- 재개 조건: 없음 - -## 리뷰어를 위한 체크포인트 - -- Implementation waited for predecessor `01_scan_create_ledger` completion. -- `identity_backfilled` is durable and ordered after `develop_matched`. -- Backfill commit stages only the target Milestone identity block and records ledger only after push success. - -## 검증 결과 - -_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ - -### API-1 중간 검증 -```sh -$ cd services/core && go test -count=1 ./internal/roadmapsync ./internal/storage -(output) -``` - -### API-2 중간 검증 -```sh -$ cd services/core && go test -count=1 ./internal/roadmapsyncpipeline ./internal/authoring -(output) -``` - -### API-3 중간 검증 -```sh -$ cd services/core && go test -count=1 ./internal/scheduler ./cmd/server ./internal/roadmapsyncpipeline -(output) -``` - -### 최종 검증 -```sh -$ git diff --check -(output) -$ cd services/core && go test -count=1 ./internal/roadmapsync ./internal/storage ./internal/authoring ./internal/roadmapsyncpipeline ./internal/scheduler ./cmd/server -(output) -$ cd services/core && go vet ./internal/roadmapsync ./internal/storage ./internal/authoring ./internal/roadmapsyncpipeline ./internal/scheduler ./cmd/server -(output) -``` - ---- - -> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** diff --git a/services/core/cmd/server/main.go b/services/core/cmd/server/main.go index 6f15820..2e18a34 100644 --- a/services/core/cmd/server/main.go +++ b/services/core/cmd/server/main.go @@ -104,6 +104,18 @@ func run(logger *slog.Logger) error { // self-actor guard is active only when PLANE_SELF_ACTOR_ID is configured. creationSync := roadmapsyncpipeline.NewService(store, planeClient, cfg.PlaneSelfActorID) + // The identity backfiller is only wired when a local develop checkout is + // configured; without it, a missing-create identity backfill fails + // explicitly (see Service.SetBackfiller) instead of silently completing + // with only the DB identity. + if cfg.GitoDevelopRepoPath != "" { + backfiller, err := roadmapsyncpipeline.NewGitBackfiller(cfg.GitoDevelopRepoPath, cfg.GitoRemoteName, cfg.GitoBranch) + if err != nil { + return err + } + creationSync.SetBackfiller(backfiller) + } + workflowService := workflow.NewService(store, nil, logger) identityWriter := authoring.NewLocalIdentityWriter() diff --git a/services/core/cmd/server/main_test.go b/services/core/cmd/server/main_test.go index 0eb025b..f3db602 100644 --- a/services/core/cmd/server/main_test.go +++ b/services/core/cmd/server/main_test.go @@ -15,6 +15,7 @@ import ( "github.com/nomadcode/nomadcode-core/internal/gitosync" apphttp "github.com/nomadcode/nomadcode-core/internal/http" "github.com/nomadcode/nomadcode-core/internal/roadmapsync" + "github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline" "github.com/nomadcode/nomadcode-core/internal/scheduler" "github.com/nomadcode/nomadcode-core/internal/workitem" ) @@ -386,3 +387,23 @@ func TestGitoHTTPWebhookDeliveryOffTargetIgnored(t *testing.T) { t.Fatalf("response status: got %q, want %q", resp["status"], "ignored") } } + +// TestCreationSyncBackfillerWiredWhenGitoCheckoutConfigured exercises the same +// construction branch run() uses to wire roadmapsyncpipeline's identity +// backfiller (see the `cfg.GitoDevelopRepoPath != ""` block in run()): a +// configured local develop checkout must produce a usable GitBackfiller, and +// an empty checkout path must be rejected so a misconfigured deployment fails +// fast instead of silently running with no backfiller. +func TestCreationSyncBackfillerWiredWhenGitoCheckoutConfigured(t *testing.T) { + svc := roadmapsyncpipeline.NewService(nil, nil, "") + + backfiller, err := roadmapsyncpipeline.NewGitBackfiller(t.TempDir(), "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller with configured checkout path: %v", err) + } + svc.SetBackfiller(backfiller) + + if _, err := roadmapsyncpipeline.NewGitBackfiller("", "origin", "develop"); err == nil { + t.Fatal("expected NewGitBackfiller to reject an empty develop checkout path") + } +} diff --git a/services/core/internal/authoring/identity_write.go b/services/core/internal/authoring/identity_write.go index eefb416..42876dd 100644 --- a/services/core/internal/authoring/identity_write.go +++ b/services/core/internal/authoring/identity_write.go @@ -99,25 +99,7 @@ func (w *LocalIdentityWriter) EnsureProviderIdentity(ctx context.Context, in Ens } if errors.Is(err, roadmapsync.ErrNoProviderIdentity) { - var sb strings.Builder - sb.WriteString("\n\n## Provider identity\n") - if in.Identity.Provider != "" { - sb.WriteString(fmt.Sprintf("- provider: %s\n", in.Identity.Provider)) - } - if in.Identity.Tenant != "" { - sb.WriteString(fmt.Sprintf("- tenant: %s\n", in.Identity.Tenant)) - } - if in.Identity.Project != "" { - sb.WriteString(fmt.Sprintf("- project: %s\n", in.Identity.Project)) - } - if in.Identity.WorkItemID != "" { - sb.WriteString(fmt.Sprintf("- work item id: %s\n", in.Identity.WorkItemID)) - } - if in.Identity.RoadmapItemID != "" { - sb.WriteString(fmt.Sprintf("- milestone id: %s\n", in.Identity.RoadmapItemID)) - } - - newContent := content + sb.String() + newContent := content + RenderProviderIdentityBlock(in.Identity) err = os.WriteFile(fullPath, []byte(newContent), 0644) if err != nil { return EnsureProviderIdentityResult{}, fmt.Errorf("write file %s failed: %w", f, err) @@ -132,3 +114,28 @@ func (w *LocalIdentityWriter) EnsureProviderIdentity(ctx context.Context, in Ens InjectedPaths: injected, }, nil } + +// RenderProviderIdentityBlock renders the "## Provider identity" Markdown +// block for an identity, in the exact shape ParseMilestoneIdentity expects. +// It makes no dirty-worktree or git assumptions, so both the authoring +// workspace writer above and a Core-authored backfill commit can reuse it. +func RenderProviderIdentityBlock(identity roadmapsync.Identity) string { + var sb strings.Builder + sb.WriteString("\n\n## Provider identity\n") + if identity.Provider != "" { + sb.WriteString(fmt.Sprintf("- provider: %s\n", identity.Provider)) + } + if identity.Tenant != "" { + sb.WriteString(fmt.Sprintf("- tenant: %s\n", identity.Tenant)) + } + if identity.Project != "" { + sb.WriteString(fmt.Sprintf("- project: %s\n", identity.Project)) + } + if identity.WorkItemID != "" { + sb.WriteString(fmt.Sprintf("- work item id: %s\n", identity.WorkItemID)) + } + if identity.RoadmapItemID != "" { + sb.WriteString(fmt.Sprintf("- milestone id: %s\n", identity.RoadmapItemID)) + } + return sb.String() +} diff --git a/services/core/internal/roadmapsync/retry.go b/services/core/internal/roadmapsync/retry.go index b260367..01131d6 100644 --- a/services/core/internal/roadmapsync/retry.go +++ b/services/core/internal/roadmapsync/retry.go @@ -9,6 +9,7 @@ type Step string const ( StepDevelopMatched Step = "develop_matched" + StepIdentityBackfilled Step = "identity_backfilled" StepOriginalCommentPreserved Step = "original_comment_preserved" StepPlaneBodyUpdated Step = "plane_body_updated" StepPlaneTodoMoved Step = "plane_todo_moved" @@ -18,6 +19,7 @@ const ( // step not present in the completed set. var OrderedSteps = []Step{ StepDevelopMatched, + StepIdentityBackfilled, StepOriginalCommentPreserved, StepPlaneBodyUpdated, StepPlaneTodoMoved, diff --git a/services/core/internal/roadmapsync/retry_test.go b/services/core/internal/roadmapsync/retry_test.go index 5bdaa26..521aff0 100644 --- a/services/core/internal/roadmapsync/retry_test.go +++ b/services/core/internal/roadmapsync/retry_test.go @@ -32,8 +32,8 @@ func TestReconcile_SamePlaneTicketReusesIdentity(t *testing.T) { 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) + if res.NextStep != StepIdentityBackfilled { + t.Errorf("next step: got %q, want identity_backfilled", res.NextStep) } } @@ -169,6 +169,7 @@ func TestReconcile_AllStepsCompleteReturnsComplete(t *testing.T) { Existing: ExistingIdentity{Found: true, Identity: id}, CompletedSteps: map[Step]bool{ StepDevelopMatched: true, + StepIdentityBackfilled: true, StepOriginalCommentPreserved: true, StepPlaneBodyUpdated: true, StepPlaneTodoMoved: true, diff --git a/services/core/internal/roadmapsyncpipeline/git_backfiller.go b/services/core/internal/roadmapsyncpipeline/git_backfiller.go new file mode 100644 index 0000000..ffe74d9 --- /dev/null +++ b/services/core/internal/roadmapsyncpipeline/git_backfiller.go @@ -0,0 +1,270 @@ +package roadmapsyncpipeline + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/nomadcode/nomadcode-core/internal/authoring" + "github.com/nomadcode/nomadcode-core/internal/roadmapsync" +) + +// commitMarker tags a backfill commit as Core-authored so a later self-loop +// check can recognize its own commit and skip re-processing it as a fresh +// authoring trigger. +const commitMarker = "NomadCode-Identity-Backfill: true" + +// ErrIdentityBackfillMismatch is returned when the target Milestone file +// already carries a provider identity block that disagrees with the identity +// being backfilled, so writing over it would silently rebind history. +var ErrIdentityBackfillMismatch = errors.New("roadmapsyncpipeline: existing provider identity does not match backfill target") + +// GitBackfiller is the production Backfiller. It bases every backfill on the +// scan-verified remote develop revision, never a stale local checkout: it +// fetches the remote branch, injects the provider identity block into the +// target Milestone content as read at that revision, stages only that file, +// verifies the staged diff is scoped to the identity block, commits with the +// Core marker on top of the verified revision, and pushes to the configured +// remote/branch. The already-satisfied and mismatch decisions are made against +// the pushed remote state, so a retry after a failed scope-check or push never +// reports success until the identity block actually landed on the remote +// branch. +type GitBackfiller struct { + RepoPath string + RemoteName string + Branch string +} + +// NewGitBackfiller builds a GitBackfiller. RepoPath is required; RemoteName +// and Branch default to "origin" and "develop" when empty. +func NewGitBackfiller(repoPath, remoteName, branch string) (*GitBackfiller, error) { + if strings.TrimSpace(repoPath) == "" { + return nil, errors.New("roadmapsyncpipeline: backfiller repo path is required") + } + if strings.TrimSpace(remoteName) == "" { + remoteName = "origin" + } + if strings.TrimSpace(branch) == "" { + branch = "develop" + } + return &GitBackfiller{RepoPath: repoPath, RemoteName: remoteName, Branch: branch}, nil +} + +// BackfillIdentity injects the provider identity block into the target +// Milestone as read at the scan-verified remote develop revision, stages only +// that path, verifies the staged diff adds nothing but the identity block, +// commits with the Core marker on top of the verified revision, and pushes to +// develop. It fetches the remote branch first so the already-satisfied and +// mismatch decisions and the commit base all use the pushed remote state, and +// refuses to touch a dirty checkout or a revision that has moved since the scan. +func (b *GitBackfiller) BackfillIdentity(ctx context.Context, in BackfillInput) error { + path := strings.TrimSpace(in.MarkdownPath) + if path == "" { + path = in.Identity.RoadmapMilestonePath + } + if path == "" { + return errors.New("roadmapsyncpipeline: backfill markdown path is required") + } + + remoteRef := b.RemoteName + "/" + b.Branch + + // Fetch the remote target branch so every decision below observes the pushed + // develop state the scanner verified, never a stale local checkout or an + // unpushed local commit. + if err := b.run(ctx, "fetch", "--prune", b.RemoteName, b.Branch); err != nil { + return err + } + + remoteRev, err := b.output(ctx, "rev-parse", remoteRef) + if err != nil { + return err + } + remoteRev = strings.TrimSpace(remoteRev) + if remoteRev == "" { + return fmt.Errorf("roadmapsyncpipeline: empty remote revision for %s", remoteRef) + } + + // If the caller passed the scan-verified revision, the remote ref must still + // point at it. Otherwise develop moved since the scan and a fresh event + // should re-drive the sync rather than committing on top of a revision the + // scanner never checked. + if expected := strings.TrimSpace(in.RoadmapRevision); expected != "" && expected != remoteRev { + return fmt.Errorf("roadmapsyncpipeline: backfill target revision moved: %s is at %s, verified scan revision was %s", remoteRef, remoteRev, expected) + } + + // Refuse to mutate a dirty checkout: an unrelated in-flight edit must never + // ride along in a Core-authored backfill commit, and resetting the base to + // the remote revision would otherwise silently discard that work. + dirty, err := b.output(ctx, "status", "--porcelain") + if err != nil { + return err + } + if strings.TrimSpace(dirty) != "" { + return fmt.Errorf("roadmapsyncpipeline: backfill checkout is dirty, refusing to mutate: %s", strings.TrimSpace(dirty)) + } + + // Read the milestone content at the verified remote revision. The + // already-satisfied and mismatch decisions are made against the pushed + // remote state, so a retry after a failed push never reports success until + // the identity block actually landed on the remote branch. + remoteContent, err := b.output(ctx, "show", remoteRev+":"+path) + if err != nil { + return fmt.Errorf("roadmapsyncpipeline: read milestone at %s failed: %w", remoteRev, err) + } + + existing, parseErr := roadmapsync.ParseMilestoneIdentity(path, remoteContent) + if parseErr == nil { + if identityMatches(existing, in.Identity) { + // The identity block already exists on the remote develop branch; + // nothing left to commit (e.g. a prior push landed but the ledger + // mark failed). + return nil + } + return ErrIdentityBackfillMismatch + } + if !errors.Is(parseErr, roadmapsync.ErrNoProviderIdentity) { + return fmt.Errorf("roadmapsyncpipeline: parse milestone identity failed: %w", parseErr) + } + + // Base the backfill commit on the verified remote revision, not the local + // HEAD, so a checkout that is behind origin/develop still commits on top of + // the revision the scanner verified. The dirty guard above already ensured + // this force-reset discards no uncommitted work. + if err := b.run(ctx, "checkout", "-B", b.Branch, remoteRev); err != nil { + return err + } + + block := authoring.RenderProviderIdentityBlock(in.Identity) + fullPath := filepath.Join(b.RepoPath, path) + if err := os.WriteFile(fullPath, []byte(remoteContent+block), 0644); err != nil { + return b.cleanup(ctx, remoteRev, fmt.Errorf("roadmapsyncpipeline: write milestone file failed: %w", err)) + } + + if err := b.run(ctx, "add", "--", path); err != nil { + return b.cleanup(ctx, remoteRev, err) + } + + if err := b.verifyScopedDiff(ctx, path, block); err != nil { + return b.cleanup(ctx, remoteRev, err) + } + + commitMsg := fmt.Sprintf("chore(roadmap): backfill provider identity for %s\n\n%s", in.Identity.MilestoneID(), commitMarker) + if err := b.run(ctx, "commit", "-m", commitMsg); err != nil { + return b.cleanup(ctx, remoteRev, err) + } + + if err := b.run(ctx, "push", b.RemoteName, "HEAD:"+b.Branch); err != nil { + // Leave the local commit in place and surface the error: the ledger step + // is only marked by the caller after this returns nil, so a failed push + // never advances the ledger, and a retry re-bases on the freshly fetched + // remote revision (checkout -B above) before pushing again. + return err + } + + return nil +} + +// cleanup restores the checkout to rev after a pre-push failure so a later +// retry re-bases from the remote revision instead of finding a staged or +// half-written identity block it might mistake for progress. The original +// cause is preserved; a cleanup failure is appended rather than hiding it. +func (b *GitBackfiller) cleanup(ctx context.Context, rev string, cause error) error { + if err := b.run(ctx, "reset", "--hard", rev); err != nil { + return fmt.Errorf("%w; additionally backfill cleanup failed: %v", cause, err) + } + return cause +} + +// identityMatches reports whether an existing identity block is the same +// provider-side identity as the one being backfilled. It compares the full +// provider key — provider, tenant, project, work item id — plus the canonical +// milestone id, so a block that reuses the same work item under a different +// milestone id is treated as a mismatch rather than silently accepted. +func identityMatches(a, b roadmapsync.Identity) bool { + return a.Provider == b.Provider && + a.Tenant == b.Tenant && + a.Project == b.Project && + a.WorkItemID == b.WorkItemID && + a.MilestoneID() == b.MilestoneID() +} + +// verifyScopedDiff ensures the staged change touches only the target path and +// that the path's diff is a pure addition matching the injected identity +// block, so an unrelated dirty-workspace roadmap edit never rides along in a +// Core-authored backfill commit. +func (b *GitBackfiller) verifyScopedDiff(ctx context.Context, path, block string) error { + staged, err := b.output(ctx, "diff", "--cached", "--name-only") + if err != nil { + return err + } + files := nonEmptyLines(staged) + if len(files) != 1 || files[0] != path { + return fmt.Errorf("roadmapsyncpipeline: backfill commit scope check failed: staged files=%v, want only %q", files, path) + } + + diff, err := b.output(ctx, "diff", "--cached", "--", path) + if err != nil { + return err + } + return verifyIdentityOnlyDiff(diff, block) +} + +// verifyIdentityOnlyDiff rejects a staged diff that removes any line or adds +// a line outside the rendered identity block. +func verifyIdentityOnlyDiff(diff, block string) error { + blockLines := strings.Split(strings.TrimRight(block, "\n"), "\n") + blockSet := make(map[string]bool, len(blockLines)) + for _, l := range blockLines { + blockSet[l] = true + } + + for _, line := range strings.Split(diff, "\n") { + switch { + case strings.HasPrefix(line, "+++") || strings.HasPrefix(line, "---"): + continue + case strings.HasPrefix(line, "-"): + return fmt.Errorf("roadmapsyncpipeline: backfill commit scope check failed: unexpected removed line %q", line) + case strings.HasPrefix(line, "+"): + added := line[1:] + if !blockSet[added] { + return fmt.Errorf("roadmapsyncpipeline: backfill commit scope check failed: unexpected added line %q", added) + } + } + } + return nil +} + +func (b *GitBackfiller) run(ctx context.Context, args ...string) error { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = b.RepoPath + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("roadmapsyncpipeline: git %s failed: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(string(out))) + } + return nil +} + +func (b *GitBackfiller) output(ctx context.Context, args ...string) (string, error) { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = b.RepoPath + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("roadmapsyncpipeline: git %s failed: %w", strings.Join(args, " "), err) + } + return string(out), nil +} + +func nonEmptyLines(s string) []string { + var out []string + for _, l := range strings.Split(s, "\n") { + l = strings.TrimSpace(l) + if l != "" { + out = append(out, l) + } + } + return out +} diff --git a/services/core/internal/roadmapsyncpipeline/git_backfiller_test.go b/services/core/internal/roadmapsyncpipeline/git_backfiller_test.go new file mode 100644 index 0000000..321d599 --- /dev/null +++ b/services/core/internal/roadmapsyncpipeline/git_backfiller_test.go @@ -0,0 +1,392 @@ +package roadmapsyncpipeline + +import ( + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/nomadcode/nomadcode-core/internal/roadmapsync" +) + +// setupBackfillRepos creates a bare "remote" repo and a working clone with an +// initial commit on branch so BackfillIdentity has something real to push to. +func setupBackfillRepos(t *testing.T, branch string) (workDir string) { + t.Helper() + workDir, _ = setupBackfillReposWithRemote(t, branch) + return workDir +} + +// setupBackfillReposWithRemote is setupBackfillRepos but also returns the bare +// remote path, so tests that need to advance origin/develop or break the push +// URL can reach it. +func setupBackfillReposWithRemote(t *testing.T, branch string) (workDir, remoteDir string) { + t.Helper() + + remoteDir, err := os.MkdirTemp("", "nomadcode-backfill-remote-*") + if err != nil { + t.Fatalf("failed to create remote dir: %v", err) + } + t.Cleanup(func() { os.RemoveAll(remoteDir) }) + runGit(t, remoteDir, "init", "--bare") + + workDir, err = os.MkdirTemp("", "nomadcode-backfill-work-*") + if err != nil { + t.Fatalf("failed to create work dir: %v", err) + } + t.Cleanup(func() { os.RemoveAll(workDir) }) + + runGit(t, workDir, "init", "-b", branch) + runGit(t, workDir, "config", "user.name", "test") + runGit(t, workDir, "config", "user.email", "test@test.com") + runGit(t, workDir, "remote", "add", "origin", remoteDir) + + milestoneDir := filepath.Join(workDir, "agent-roadmap", "phase", "p1", "milestones") + if err := os.MkdirAll(milestoneDir, 0755); err != nil { + t.Fatalf("failed to create milestone dir: %v", err) + } + milestonePath := filepath.Join(milestoneDir, "m1.md") + if err := os.WriteFile(milestonePath, []byte("# Milestone\nSome description.\n"), 0644); err != nil { + t.Fatalf("failed to write milestone file: %v", err) + } + runGit(t, workDir, "add", "-A") + runGit(t, workDir, "commit", "-m", "initial") + runGit(t, workDir, "push", "origin", branch) + + return workDir, remoteDir +} + +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s failed: %v: %s", strings.Join(args, " "), err, string(out)) + } +} + +// gitOut runs a git command and returns its trimmed stdout, failing the test on +// error. It is the read-side counterpart to runGit for revision assertions. +func gitOut(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + t.Fatalf("git %s failed: %v", strings.Join(args, " "), err) + } + return strings.TrimSpace(string(out)) +} + +// advanceDevelop appends marker to the milestone file, commits, and pushes so +// origin/develop moves forward, then returns the new origin/develop revision. +func advanceDevelop(t *testing.T, workDir, path, marker string) string { + t.Helper() + fullPath := filepath.Join(workDir, path) + content, err := os.ReadFile(fullPath) + if err != nil { + t.Fatalf("failed to read milestone file: %v", err) + } + if err := os.WriteFile(fullPath, []byte(string(content)+marker), 0644); err != nil { + t.Fatalf("failed to advance milestone file: %v", err) + } + runGit(t, workDir, "add", "-A") + runGit(t, workDir, "commit", "-m", "advance develop") + runGit(t, workDir, "push", "origin", "develop") + return gitOut(t, workDir, "rev-parse", "origin/develop") +} + +func testBackfillIdentity() roadmapsync.Identity { + return roadmapsync.Identity{ + Provider: "plane", + WorkItemID: "NOMAD-1", + Tenant: "general", + Project: "nomad", + RoadmapItemID: "m1", + } +} + +func TestGitBackfillerCommitsAndPushesIdentityOnly(t *testing.T) { + workDir := setupBackfillRepos(t, "develop") + b, err := NewGitBackfiller(workDir, "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller failed: %v", err) + } + + path := "agent-roadmap/phase/p1/milestones/m1.md" + if err := b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + }); err != nil { + t.Fatalf("BackfillIdentity failed: %v", err) + } + + content, err := os.ReadFile(filepath.Join(workDir, path)) + if err != nil { + t.Fatalf("failed to read milestone file: %v", err) + } + parsed, err := roadmapsync.ParseMilestoneIdentity(path, string(content)) + if err != nil { + t.Fatalf("failed to parse injected identity: %v", err) + } + if parsed.Provider != "plane" || parsed.WorkItemID != "NOMAD-1" || parsed.Tenant != "general" || parsed.Project != "nomad" { + t.Errorf("unexpected parsed identity: %+v", parsed) + } + + // Verify the push actually reached the bare remote. + out, err := exec.Command("git", "-C", workDir, "log", "origin/develop", "-1", "--format=%s%n%b").CombinedOutput() + if err != nil { + t.Fatalf("git log origin/develop failed: %v: %s", err, string(out)) + } + if !strings.Contains(string(out), commitMarker) { + t.Errorf("expected pushed commit to carry Core marker, got: %s", string(out)) + } + + // A second call after a successful push must be a no-op (idempotent retry + // after a ledger-mark failure), not a duplicate commit. + headBefore, err := exec.Command("git", "-C", workDir, "rev-parse", "HEAD").Output() + if err != nil { + t.Fatalf("rev-parse failed: %v", err) + } + if err := b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + }); err != nil { + t.Fatalf("second BackfillIdentity call failed: %v", err) + } + headAfter, err := exec.Command("git", "-C", workDir, "rev-parse", "HEAD").Output() + if err != nil { + t.Fatalf("rev-parse failed: %v", err) + } + if string(headBefore) != string(headAfter) { + t.Errorf("expected no new commit on retry, HEAD moved from %s to %s", headBefore, headAfter) + } +} + +// TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted proves the backfill +// commit stays scoped to the injected identity block: an unrelated dirty +// change elsewhere in the same milestone file must abort the backfill rather +// than being swept into the Core-authored commit. +func TestGitBackfillerLeavesUnrelatedDirtyChangeUncommitted(t *testing.T) { + workDir := setupBackfillRepos(t, "develop") + path := "agent-roadmap/phase/p1/milestones/m1.md" + fullPath := filepath.Join(workDir, path) + + // Simulate an unrelated in-flight edit to the same file that must not + // ride along in the backfill commit. + original, err := os.ReadFile(fullPath) + if err != nil { + t.Fatalf("failed to read milestone file: %v", err) + } + dirty := string(original) + "\nUnrelated roadmap edit.\n" + + b, err := NewGitBackfiller(workDir, "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller failed: %v", err) + } + + // Write the dirty content directly so BackfillIdentity's own write lands + // on top of it, exactly like a concurrently dirty workspace would. + if err := os.WriteFile(fullPath, []byte(dirty), 0644); err != nil { + t.Fatalf("failed to write dirty content: %v", err) + } + + err = b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + }) + if err == nil { + t.Fatal("expected scope check to reject the unrelated dirty change, got nil error") + } + + // The unrelated edit must remain unstaged/uncommitted: no push happened. + out, lerr := exec.Command("git", "-C", workDir, "log", "origin/develop", "-1", "--format=%s").CombinedOutput() + if lerr != nil { + t.Fatalf("git log origin/develop failed: %v: %s", lerr, string(out)) + } + if strings.TrimSpace(string(out)) != "initial" { + t.Errorf("expected origin/develop to still be at the initial commit, got %q", strings.TrimSpace(string(out))) + } +} + +// TestGitBackfillerRejectsMismatchedExistingIdentity proves the mismatch guard +// is driven by the pushed remote state: a different work item id already on +// origin/develop must abort rather than being silently rebound. +func TestGitBackfillerRejectsMismatchedExistingIdentity(t *testing.T) { + workDir := setupBackfillRepos(t, "develop") + path := "agent-roadmap/phase/p1/milestones/m1.md" + fullPath := filepath.Join(workDir, path) + + existing := "# Milestone\n\n## Provider identity\n- provider: plane\n- tenant: general\n- project: nomad\n- work item id: NOMAD-OTHER\n- milestone id: m1\n" + if err := os.WriteFile(fullPath, []byte(existing), 0644); err != nil { + t.Fatalf("failed to write existing identity: %v", err) + } + runGit(t, workDir, "add", "-A") + runGit(t, workDir, "commit", "-m", "existing identity") + // The mismatch must be recognized from the remote branch, not a local-only + // commit, so push the conflicting identity to origin/develop first. + runGit(t, workDir, "push", "origin", "develop") + + b, err := NewGitBackfiller(workDir, "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller failed: %v", err) + } + + err = b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + }) + if !errors.Is(err, ErrIdentityBackfillMismatch) { + t.Fatalf("expected ErrIdentityBackfillMismatch, got %v", err) + } +} + +// TestGitBackfillerRejectsMismatchedMilestoneID proves the mismatch guard +// includes the canonical milestone id: an identity that reuses the same +// provider/tenant/project/work item id under a different milestone id must be +// rejected rather than treated as already satisfied. +func TestGitBackfillerRejectsMismatchedMilestoneID(t *testing.T) { + workDir := setupBackfillRepos(t, "develop") + path := "agent-roadmap/phase/p1/milestones/m1.md" + fullPath := filepath.Join(workDir, path) + + existing := "# Milestone\n\n## Provider identity\n- provider: plane\n- tenant: general\n- project: nomad\n- work item id: NOMAD-1\n- milestone id: m-different\n" + if err := os.WriteFile(fullPath, []byte(existing), 0644); err != nil { + t.Fatalf("failed to write existing identity: %v", err) + } + runGit(t, workDir, "add", "-A") + runGit(t, workDir, "commit", "-m", "existing identity, other milestone") + runGit(t, workDir, "push", "origin", "develop") + + b, err := NewGitBackfiller(workDir, "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller failed: %v", err) + } + + err = b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + }) + if !errors.Is(err, ErrIdentityBackfillMismatch) { + t.Fatalf("expected ErrIdentityBackfillMismatch for milestone id mismatch, got %v", err) + } +} + +// TestGitBackfillerRetriesAfterPushFailure proves a failed push never reports +// success: the first attempt fails to push and origin/develop stays without the +// identity, and only once the push can reach the remote does a retry land it. +func TestGitBackfillerRetriesAfterPushFailure(t *testing.T) { + workDir, remoteDir := setupBackfillReposWithRemote(t, "develop") + path := "agent-roadmap/phase/p1/milestones/m1.md" + + b, err := NewGitBackfiller(workDir, "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller failed: %v", err) + } + + // Break only the push URL so the fetch still succeeds but the push fails. + brokenPush := filepath.Join(t.TempDir(), "nonexistent-remote.git") + runGit(t, workDir, "remote", "set-url", "--push", "origin", brokenPush) + + if err := b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + }); err == nil { + t.Fatal("expected push failure, got nil error") + } + + // The remote must not carry the identity block yet. + pushed := gitOut(t, workDir, "show", "origin/develop:"+path) + if _, perr := roadmapsync.ParseMilestoneIdentity(path, pushed); perr == nil { + t.Fatal("expected origin/develop to lack the identity after a failed push") + } + + // Restore the push URL and retry: now it must land on the remote. + runGit(t, workDir, "remote", "set-url", "--push", "origin", remoteDir) + if err := b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + }); err != nil { + t.Fatalf("retry after restoring push failed: %v", err) + } + pushed = gitOut(t, workDir, "show", "origin/develop:"+path) + if _, perr := roadmapsync.ParseMilestoneIdentity(path, pushed); perr != nil { + t.Fatalf("expected origin/develop to carry the identity after retry: %v", perr) + } + if out := gitOut(t, workDir, "log", "origin/develop", "-1", "--format=%b"); !strings.Contains(out, commitMarker) { + t.Errorf("expected pushed commit to carry Core marker, got: %s", out) + } +} + +// TestGitBackfillerCommitsOnVerifiedRemoteRevisionWhenLocalBehind proves the +// backfill commit is based on origin/develop even when the local checkout is +// behind it: the commit's parent is the advanced remote revision and its +// content includes the advanced develop content, not the stale local HEAD. +func TestGitBackfillerCommitsOnVerifiedRemoteRevisionWhenLocalBehind(t *testing.T) { + workDir := setupBackfillRepos(t, "develop") + path := "agent-roadmap/phase/p1/milestones/m1.md" + + // Advance origin/develop, then move the local branch behind it. + remoteRev := advanceDevelop(t, workDir, path, "\nDevelop moved ahead.\n") + runGit(t, workDir, "reset", "--hard", "HEAD~1") + + b, err := NewGitBackfiller(workDir, "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller failed: %v", err) + } + if err := b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + RoadmapRevision: remoteRev, + }); err != nil { + t.Fatalf("BackfillIdentity failed: %v", err) + } + + // The backfill commit must sit directly on top of the verified remote + // revision, not the stale local HEAD. + parent := gitOut(t, workDir, "rev-parse", "origin/develop^") + if parent != remoteRev { + t.Errorf("backfill commit parent = %s, want verified remote revision %s", parent, remoteRev) + } + pushed := gitOut(t, workDir, "show", "origin/develop:"+path) + if !strings.Contains(pushed, "Develop moved ahead.") { + t.Errorf("expected backfill to build on advanced develop content, got: %s", pushed) + } + if _, perr := roadmapsync.ParseMilestoneIdentity(path, pushed); perr != nil { + t.Errorf("expected identity block on pushed content: %v", perr) + } +} + +// TestGitBackfillerRejectsMovedRemoteRevision proves a stale verified revision +// aborts before any file mutation: if origin/develop has moved past the +// scan-verified revision, the backfill fails and the remote gains no identity. +func TestGitBackfillerRejectsMovedRemoteRevision(t *testing.T) { + workDir := setupBackfillRepos(t, "develop") + path := "agent-roadmap/phase/p1/milestones/m1.md" + + staleRev := gitOut(t, workDir, "rev-parse", "origin/develop") + // Move origin/develop forward so staleRev is no longer the tip. + advanceDevelop(t, workDir, path, "\nDevelop moved ahead.\n") + + b, err := NewGitBackfiller(workDir, "origin", "develop") + if err != nil { + t.Fatalf("NewGitBackfiller failed: %v", err) + } + err = b.BackfillIdentity(context.Background(), BackfillInput{ + Identity: testBackfillIdentity(), + MarkdownPath: path, + RoadmapRevision: staleRev, + }) + if err == nil { + t.Fatal("expected error for moved remote revision, got nil") + } + + pushed := gitOut(t, workDir, "show", "origin/develop:"+path) + if _, perr := roadmapsync.ParseMilestoneIdentity(path, pushed); perr == nil { + t.Error("expected no identity backfill after a revision-mismatch abort") + } +} diff --git a/services/core/internal/roadmapsyncpipeline/service.go b/services/core/internal/roadmapsyncpipeline/service.go index 042ef04..e921ae5 100644 --- a/services/core/internal/roadmapsyncpipeline/service.go +++ b/services/core/internal/roadmapsyncpipeline/service.go @@ -54,6 +54,27 @@ type Store interface { MarkStepCompleted(ctx context.Context, identityID int64, step storage.RoadmapSyncStep) (db.RoadmapSyncStep, error) } +// Backfiller writes the provider identity block to the develop checkout, +// commits only that block, and pushes it to develop. SyncCreation calls it +// right after a Plane work item is created (or on resume after a prior push +// failure) and only marks the identity_backfilled ledger step once it +// returns nil, so a retry never records the step for a push that did not +// actually land. +type Backfiller interface { + BackfillIdentity(ctx context.Context, in BackfillInput) error +} + +// BackfillInput carries the identity to inject, the develop-relative Milestone +// path to inject it into, and the scan-verified develop revision the backfill +// commit must be based on. RoadmapRevision is the revision the scanner verified +// on origin/develop; the backfiller refuses to commit on top of a revision that +// no longer matches it, so a Core-authored backfill never lands on stale state. +type BackfillInput struct { + Identity roadmapsync.Identity + MarkdownPath string + RoadmapRevision string +} + // Service drives the creation sync orchestration against a Store and the Plane // Todo provider facets. type Service struct { @@ -62,6 +83,11 @@ type Service struct { // selfActor identifies NomadCode's own provider account so a mutation it made // is not re-interpreted as a fresh user trigger. selfActor string + // backfiller performs the Core-authored identity backfill commit/push for + // missing-create identities. Nil means backfill is not configured; a + // missing-create cycle then fails explicitly instead of silently + // completing with only the DB identity. + backfiller Backfiller } // NewService builds the orchestrator. provider is the Plane adapter (or a fake) @@ -71,6 +97,13 @@ func NewService(store Store, provider roadmapsync.PlaneTodoProvider, selfActor s return &Service{store: store, provider: provider, selfActor: selfActor} } +// SetBackfiller configures the identity backfiller. It is a post-construction +// setter (like workflow.Service.SetEnqueuer) so callers that do not need +// missing-create identity backfill can keep using NewService unchanged. +func (s *Service) SetBackfiller(b Backfiller) { + s.backfiller = b +} + // SyncCreationInput carries everything one creation sync needs without reaching // into provider DTOs: the develop scan result and the expected work item // identity (the develop match inputs), the Plane work item ref, the original @@ -117,6 +150,13 @@ func (s *Service) SyncCreation(ctx context.Context, in SyncCreationInput) (SyncC var found bool var err error + // missingCreateShaped is computed from the trigger's original identity + // fields before the found-identity infill below overwrites them, so a + // redelivered missing-create job (no work item id/ref on the trigger) is + // still recognized as such even after an earlier cycle already created + // the Plane item and populated the persisted row. + missingCreateShaped := in.Expected.WorkItemID == "" && in.Ref.ID == "" + // Load existing identity row (without mutation) if in.Expected.MilestoneID() != "" { existingRow, completed, found, err = s.loadExistingByID(ctx, in.Expected.MilestoneID()) @@ -155,7 +195,16 @@ func (s *Service) SyncCreation(ctx context.Context, in SyncCreationInput) (SyncC if in.Ref.ID == "" { in.Ref.ID = in.Expected.WorkItemID } - } else if in.Expected.WorkItemID == "" && in.Ref.ID == "" { + + // A prior cycle already created the Plane item and upserted the + // identity (existingRow.WorkItemID set), but the identity backfill + // commit/push never completed — resume the backfill directly instead + // of falling through to develop-match, which cannot succeed until the + // backfill push actually lands the identity block on develop. + if missingCreateShaped && existingRow.WorkItemID != "" && !completed[storage.StepIdentityBackfilled] { + return s.backfillIdentityAndMark(ctx, existingRow, existingFromRow(existingRow).Identity, in.Scan.Revision.Normalize().Revision) + } + } else if missingCreateShaped { // Plane missing-create scenario: no existing work item id means the // active milestone has no Plane ticket yet. Expected.Provider is set by // the bridge (e.g. "plane"); fall back to "plane" only when the caller @@ -224,11 +273,7 @@ func (s *Service) SyncCreation(ctx context.Context, in SyncCreationInput) (SyncC return SyncCreationResult{}, err } - return SyncCreationResult{ - Action: SyncActionProjected, - Reason: "plane work item created for missing active milestone", - NextStep: roadmapsync.StepDevelopMatched, - }, nil + return s.backfillIdentityAndMark(ctx, identityRow, identity, in.Scan.Revision.Normalize().Revision) } candidate := roadmapsync.MatchDevelopMilestone(roadmapsync.MatchInput{ @@ -420,6 +465,28 @@ func (s *Service) loadExistingByID(ctx context.Context, milestoneID string) (db. return row, completed, true, nil } +// backfillIdentityAndMark runs the Core-authored identity backfill commit/push +// for identityRow and, only once it succeeds, marks the identity_backfilled +// ledger step. A missing backfiller fails explicitly rather than silently +// completing with only the DB identity, so a misconfigured deployment cannot +// mistake a DB-only write for a durable, provider-verifiable identity. +func (s *Service) backfillIdentityAndMark(ctx context.Context, identityRow db.RoadmapSyncIdentity, identity roadmapsync.Identity, roadmapRevision string) (SyncCreationResult, error) { + if s.backfiller == nil { + return SyncCreationResult{}, errors.New("roadmapsyncpipeline: identity backfiller not configured") + } + if err := s.backfiller.BackfillIdentity(ctx, BackfillInput{Identity: identity, MarkdownPath: identity.RoadmapMilestonePath, RoadmapRevision: roadmapRevision}); err != nil { + return SyncCreationResult{}, err + } + if _, err := s.store.MarkStepCompleted(ctx, identityRow.ID, toStorageStep(roadmapsync.StepIdentityBackfilled)); err != nil { + return SyncCreationResult{}, err + } + return SyncCreationResult{ + Action: SyncActionProjected, + Reason: "provider identity backfilled to develop", + NextStep: roadmapsync.StepIdentityBackfilled, + }, nil +} + // runRemainingSteps executes the creation-cycle steps from nextStep onward, // skipping any already in the completed set, and records a ledger mark right // after each provider call succeeds. A provider failure stops the sequence so a @@ -453,10 +520,15 @@ func (s *Service) runRemainingSteps(ctx context.Context, identityID int64, nextS // runStep performs the provider side effect for a single creation-cycle step. // StepDevelopMatched has no provider side effect — it only records that the // develop match gate passed — so it is a no-op here and the caller still records -// its ledger mark. +// its ledger mark. StepIdentityBackfilled is likewise a no-op here: by the time +// this general resume loop reaches it, the develop scan already matched the +// candidate against a Milestone doc that carries the identity block, so the +// content backfill Core would otherwise perform is already satisfied; the +// dedicated missing-create/resume path in SyncCreation is the only place that +// actually calls the Backfiller. func (s *Service) runStep(ctx context.Context, step roadmapsync.Step, in roadmapsync.ProjectPlaneTodoInput) error { switch step { - case roadmapsync.StepDevelopMatched: + case roadmapsync.StepDevelopMatched, roadmapsync.StepIdentityBackfilled: return nil case roadmapsync.StepOriginalCommentPreserved: return roadmapsync.PreserveOriginalComment(ctx, s.provider, in) diff --git a/services/core/internal/roadmapsyncpipeline/service_test.go b/services/core/internal/roadmapsyncpipeline/service_test.go index 99599ec..5234a39 100644 --- a/services/core/internal/roadmapsyncpipeline/service_test.go +++ b/services/core/internal/roadmapsyncpipeline/service_test.go @@ -139,6 +139,23 @@ func (f *fakeStore) MarkStepCompleted(_ context.Context, _ int64, step storage.R return db.RoadmapSyncStep{Step: string(step)}, nil } +// fakeBackfiller records the calls BackfillIdentity received and can fail on +// demand, so tests can assert call order (create -> identity upsert -> +// develop_matched mark -> backfill -> identity_backfilled mark) and retry +// behavior without a real git checkout. +type fakeBackfiller struct { + calls []BackfillInput + err error +} + +func (f *fakeBackfiller) BackfillIdentity(_ context.Context, in BackfillInput) error { + f.calls = append(f.calls, in) + if f.err != nil { + return f.err + } + return nil +} + func readyInput() SyncCreationInput { identity := roadmapsync.Identity{ Shape: roadmapsync.ShapeMilestone, @@ -213,6 +230,7 @@ func TestSyncCreationReadyRunsStepsAndMarksLedger(t *testing.T) { wantLedger := []storage.RoadmapSyncStep{ storage.StepDevelopMatched, + storage.StepIdentityBackfilled, storage.StepOriginalCommentPreserved, storage.StepPlaneBodyUpdated, storage.StepPlaneTodoMoved, @@ -264,6 +282,7 @@ func TestSyncCreationRetrySkipsCompletedSteps(t *testing.T) { t.Fatalf("provider call order = %v, want %v", provider.calls, wantProviderOrder) } wantLedger := []storage.RoadmapSyncStep{ + storage.StepIdentityBackfilled, storage.StepPlaneBodyUpdated, storage.StepPlaneTodoMoved, } @@ -288,9 +307,11 @@ func TestSyncCreationStopsBeforeLaterStepOnProviderError(t *testing.T) { if !equalStrings(provider.calls, wantProviderOrder) { t.Fatalf("provider call order = %v, want %v", provider.calls, wantProviderOrder) } - // develop_matched + original_comment_preserved recorded; body never marked. + // develop_matched + identity_backfilled + original_comment_preserved + // recorded; body never marked. wantLedger := []storage.RoadmapSyncStep{ storage.StepDevelopMatched, + storage.StepIdentityBackfilled, storage.StepOriginalCommentPreserved, } if !equalSteps(store.marked, wantLedger) { @@ -423,7 +444,9 @@ func equalSteps(a, b []storage.RoadmapSyncStep) bool { func TestSyncCreationMissingActiveMilestoneCreatesWorkItemAndRecordsLedger(t *testing.T) { store := newFakeStore() provider := &fakeProvider{} + backfiller := &fakeBackfiller{} svc := NewService(store, provider, "") + svc.SetBackfiller(backfiller) // 1. Prepare input representing missing active milestone (expected identity without provider details) in := readyInput() @@ -445,6 +468,9 @@ func TestSyncCreationMissingActiveMilestoneCreatesWorkItemAndRecordsLedger(t *te if res.Action != SyncActionProjected { t.Fatalf("expected projected action, got %s (%s)", res.Action, res.Reason) } + if res.NextStep != roadmapsync.StepIdentityBackfilled { + t.Fatalf("expected next step identity_backfilled, got %s", res.NextStep) + } // Fake creator should be called once with "create" wantCalls := []string{"create"} @@ -452,6 +478,21 @@ func TestSyncCreationMissingActiveMilestoneCreatesWorkItemAndRecordsLedger(t *te t.Fatalf("provider calls = %v, want %v", provider.calls, wantCalls) } + // Backfiller must run after the identity is upserted, carrying the + // created work item id and the develop-relative Milestone path. + if len(backfiller.calls) != 1 { + t.Fatalf("backfiller calls = %d, want 1", len(backfiller.calls)) + } + if backfiller.calls[0].Identity.WorkItemID != "CREATED-123" { + t.Errorf("backfiller identity WorkItemID = %q, want CREATED-123", backfiller.calls[0].Identity.WorkItemID) + } + if backfiller.calls[0].MarkdownPath != testMilestonePath { + t.Errorf("backfiller MarkdownPath = %q, want %q", backfiller.calls[0].MarkdownPath, testMilestonePath) + } + if backfiller.calls[0].RoadmapRevision != testRevision { + t.Errorf("backfiller RoadmapRevision = %q, want %q", backfiller.calls[0].RoadmapRevision, testRevision) + } + // Title should be `[milestone-id] title` (in our case extract title should return Milestone from Markdown) expectedTitle := "[m1] Milestone" if provider.createdInput.Title != expectedTitle { @@ -471,22 +512,21 @@ func TestSyncCreationMissingActiveMilestoneCreatesWorkItemAndRecordsLedger(t *te wantLedger := []storage.RoadmapSyncStep{ storage.StepDevelopMatched, + storage.StepIdentityBackfilled, } if !equalSteps(store.marked, wantLedger) { t.Fatalf("ledger marks = %v, want %v", store.marked, wantLedger) } - if res.NextStep != roadmapsync.StepDevelopMatched { - t.Fatalf("expected next step to be develop_matched, got %s", res.NextStep) - } } // TestSyncCreationMissingCreateDuplicateRetryDoesNotRecreate reprocesses the // same milestone slug and revision after a prior cycle already persisted the -// identity from a successful create. The missing-create predicate keys off -// `found` (identity row exists), not off the trigger's empty WorkItemID/Ref, -// so a duplicate delivery of the same bridge-produced missing-create job must -// not call CreateWorkItem a second time even though the trigger input still -// looks like a missing-create job. +// identity from a successful create but the identity backfill commit/push had +// not completed yet. The missing-create predicate keys off `found` (identity +// row exists), not off the trigger's empty WorkItemID/Ref, so a duplicate +// delivery of the same bridge-produced missing-create job must not call +// CreateWorkItem a second time even though the trigger input still looks like +// a missing-create job; instead it resumes the identity backfill directly. func TestSyncCreationMissingCreateDuplicateRetryDoesNotRecreate(t *testing.T) { store := newFakeStore() store.found = true @@ -504,7 +544,9 @@ func TestSyncCreationMissingCreateDuplicateRetryDoesNotRecreate(t *testing.T) { store.completed[storage.StepDevelopMatched] = true provider := &fakeProvider{} + backfiller := &fakeBackfiller{} svc := NewService(store, provider, "") + svc.SetBackfiller(backfiller) // Same missing-create shaped trigger as the bridge would re-send: no // WorkItemID/Ref, same milestone slug and develop revision. @@ -546,6 +588,25 @@ func TestSyncCreationMissingCreateDuplicateRetryDoesNotRecreate(t *testing.T) { if res.Action == SyncActionProjected && res.NextStep == roadmapsync.StepDevelopMatched { t.Fatalf("expected no fresh develop_matched projection on duplicate retry, got %+v", res) } + + // The retry must resume the backfill (not recreate the Plane item) and + // mark identity_backfilled once it succeeds. + if len(backfiller.calls) != 1 { + t.Fatalf("backfiller calls = %d, want 1", len(backfiller.calls)) + } + if backfiller.calls[0].Identity.WorkItemID != "CREATED-123" { + t.Errorf("backfiller identity WorkItemID = %q, want CREATED-123", backfiller.calls[0].Identity.WorkItemID) + } + if backfiller.calls[0].RoadmapRevision != "bbbb" { + t.Errorf("backfiller RoadmapRevision = %q, want bbbb", backfiller.calls[0].RoadmapRevision) + } + wantLedger := []storage.RoadmapSyncStep{storage.StepIdentityBackfilled} + if !equalSteps(store.marked, wantLedger) { + t.Fatalf("ledger marks = %v, want %v", store.marked, wantLedger) + } + if res.Action != SyncActionProjected || res.NextStep != roadmapsync.StepIdentityBackfilled { + t.Fatalf("expected projected/identity_backfilled, got %+v", res) + } } // TestSyncCreationBridgeProducedMissingCreate tests the actual bridge-produced @@ -555,7 +616,9 @@ func TestSyncCreationMissingCreateDuplicateRetryDoesNotRecreate(t *testing.T) { func TestSyncCreationBridgeProducedMissingCreate(t *testing.T) { store := newFakeStore() provider := &fakeProvider{} + backfiller := &fakeBackfiller{} svc := NewService(store, provider, "") + svc.SetBackfiller(backfiller) // Build input exactly as bridge.enqueueDoc does for missing-create: // Provider="plane", Tenant/Project set from config, WorkItemID/Ref empty. @@ -627,10 +690,17 @@ func TestSyncCreationBridgeProducedMissingCreate(t *testing.T) { // Verify ledger mark. wantLedger := []storage.RoadmapSyncStep{ storage.StepDevelopMatched, + storage.StepIdentityBackfilled, } if !equalSteps(store.marked, wantLedger) { t.Fatalf("ledger marks = %v, want %v", store.marked, wantLedger) } + if len(backfiller.calls) != 1 { + t.Fatalf("backfiller calls = %d, want 1", len(backfiller.calls)) + } + if backfiller.calls[0].RoadmapRevision != "bbbb" { + t.Errorf("backfiller RoadmapRevision = %q, want bbbb", backfiller.calls[0].RoadmapRevision) + } } // TestSyncCreationMissingCreateRequiresTenantAndProject ensures that a @@ -840,6 +910,7 @@ func TestSyncCreationUpdateAttributesBeforeCompleteNoOp(t *testing.T) { } // All steps complete store.completed[storage.StepDevelopMatched] = true + store.completed[storage.StepIdentityBackfilled] = true store.completed[storage.StepOriginalCommentPreserved] = true store.completed[storage.StepPlaneBodyUpdated] = true store.completed[storage.StepPlaneTodoMoved] = true diff --git a/services/core/internal/storage/roadmap_sync_steps.go b/services/core/internal/storage/roadmap_sync_steps.go index 224d2a4..916c470 100644 --- a/services/core/internal/storage/roadmap_sync_steps.go +++ b/services/core/internal/storage/roadmap_sync_steps.go @@ -14,6 +14,7 @@ type RoadmapSyncStep string const ( StepDevelopMatched RoadmapSyncStep = "develop_matched" + StepIdentityBackfilled RoadmapSyncStep = "identity_backfilled" StepOriginalCommentPreserved RoadmapSyncStep = "original_comment_preserved" StepPlaneBodyUpdated RoadmapSyncStep = "plane_body_updated" StepPlaneTodoMoved RoadmapSyncStep = "plane_todo_moved" @@ -23,6 +24,7 @@ const ( // them, so a resume can pick the first not-yet-completed step. var OrderedRoadmapSyncSteps = []RoadmapSyncStep{ StepDevelopMatched, + StepIdentityBackfilled, StepOriginalCommentPreserved, StepPlaneBodyUpdated, StepPlaneTodoMoved, @@ -36,7 +38,7 @@ 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: + case StepDevelopMatched, StepIdentityBackfilled, StepOriginalCommentPreserved, StepPlaneBodyUpdated, StepPlaneTodoMoved: return true default: return false diff --git a/services/core/internal/storage/roadmap_sync_steps_test.go b/services/core/internal/storage/roadmap_sync_steps_test.go index d39261f..0a61949 100644 --- a/services/core/internal/storage/roadmap_sync_steps_test.go +++ b/services/core/internal/storage/roadmap_sync_steps_test.go @@ -134,6 +134,22 @@ func TestMarkStepCompleted_CompletedStepReapplyIsNoOp(t *testing.T) { } } +func TestMarkStepCompleted_IdentityBackfilledStepPersists(t *testing.T) { + completedAt := time.Now() + m := &stepMockDBTX{markRow: db.RoadmapSyncStep{ + ID: 2, RoadmapSyncIdentityID: 7, Step: string(StepIdentityBackfilled), CompletedAt: completedAt, + }} + store := newStepStore(m) + + row, err := store.MarkStepCompleted(context.Background(), 7, StepIdentityBackfilled) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if row.Step != string(StepIdentityBackfilled) { + t.Errorf("row not preserved: %+v", row) + } +} + func TestMarkStepCompleted_UnknownStepRejected(t *testing.T) { m := &stepMockDBTX{} store := newStepStore(m) @@ -152,7 +168,8 @@ func TestMarkStepCompleted_UnknownStepRejected(t *testing.T) { 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)}, + {ID: 2, RoadmapSyncIdentityID: 7, Step: string(StepIdentityBackfilled)}, + {ID: 3, RoadmapSyncIdentityID: 7, Step: string(StepOriginalCommentPreserved)}, }} store := newStepStore(m) @@ -160,8 +177,8 @@ func TestCompletedSteps_ReturnsSetForResume(t *testing.T) { 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[StepDevelopMatched] || !done[StepIdentityBackfilled] || !done[StepOriginalCommentPreserved] { + t.Errorf("expected first three steps completed, got %v", done) } if done[StepPlaneBodyUpdated] || done[StepPlaneTodoMoved] { t.Errorf("uncompleted steps must be absent, got %v", done) diff --git a/services/core/migrations/00010_add_roadmap_identity_backfilled_step.sql b/services/core/migrations/00010_add_roadmap_identity_backfilled_step.sql new file mode 100644 index 0000000..b41545f --- /dev/null +++ b/services/core/migrations/00010_add_roadmap_identity_backfilled_step.sql @@ -0,0 +1,21 @@ +-- +goose Up +-- +goose StatementBegin +ALTER TABLE roadmap_sync_steps + DROP CONSTRAINT roadmap_sync_steps_step_check; +-- +goose StatementEnd +-- +goose StatementBegin +ALTER TABLE roadmap_sync_steps + ADD CONSTRAINT roadmap_sync_steps_step_check + CHECK (step IN ('develop_matched', 'identity_backfilled', 'original_comment_preserved', 'plane_body_updated', 'plane_todo_moved')); +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +ALTER TABLE roadmap_sync_steps + DROP CONSTRAINT roadmap_sync_steps_step_check; +-- +goose StatementEnd +-- +goose StatementBegin +ALTER TABLE roadmap_sync_steps + ADD CONSTRAINT roadmap_sync_steps_step_check + CHECK (step IN ('develop_matched', 'original_comment_preserved', 'plane_body_updated', 'plane_todo_moved')); +-- +goose StatementEnd