diff --git a/agent-task/m-webhook-revision-reconcile/CODE_REVIEW-local-G05.md b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_0.log similarity index 52% rename from agent-task/m-webhook-revision-reconcile/CODE_REVIEW-local-G05.md rename to agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_0.log index 6461e19..e4729ae 100644 --- a/agent-task/m-webhook-revision-reconcile/CODE_REVIEW-local-G05.md +++ b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_0.log @@ -53,43 +53,46 @@ task=m-webhook-revision-reconcile, plan=0, tag=API | 항목 | 완료 여부 | |------|---------| -| [API-1] Reconcile 모델과 expected revision conflict | [ ] | -| [API-2] Dry-run drift/conflict/proposal 결과 | [ ] | -| [API-3] Apply idempotency key와 side effect 1회 보장 | [ ] | +| [API-1] Reconcile 모델과 expected revision conflict | [x] | +| [API-2] Dry-run drift/conflict/proposal 결과 | [x] | +| [API-3] Apply idempotency key와 side effect 1회 보장 | [x] | ## 구현 체크리스트 -- [ ] Reconcile request/result 모델과 expected revision conflict 경로를 구현하고 `TestRuntimeReconcileExpectedRevisionConflict`를 통과시킨다. -- [ ] Dry-run 결과가 drift/conflict/proposal을 반환하도록 구현하고 `TestRuntimeDryRunReconcileReportsDriftConflictProposal`를 통과시킨다. -- [ ] Apply 경로가 idempotency key로 side effect를 1회만 실행하도록 구현하고 `TestRuntimeApplyReconcileIsIdempotent`를 통과시킨다. -- [ ] 최종 검증 `cd services/core && go test ./...`와 `git diff --check`를 실행한다. -- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. +- [x] Reconcile request/result 모델과 expected revision conflict 경로를 구현하고 `TestRuntimeReconcileExpectedRevisionConflict`를 통과시켰다. +- [x] Dry-run 결과가 drift/conflict/proposal을 반환하도록 구현하고 `TestRuntimeDryRunReconcileReportsDriftConflictProposal`를 통과시켰다. +- [x] Apply 경로가 idempotency key로 side effect를 1회만 실행하도록 구현하고 `TestRuntimeApplyReconcileIsIdempotent`를 통과시켰다. +- [x] 최종 검증 `cd services/core && go test ./...`와 `git diff --check`를 실행했다. +- [x] CODE_REVIEW-*-G05.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채웠다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. ## 코드리뷰 전용 체크리스트 > **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. > 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. -- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. -- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. -- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G05_N.log`로 아카이브한다. -- [ ] active `PLAN-*-G??.md`를 `plan_local_G05_M.log`로 아카이브한다. -- [ ] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G05_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_local_G05_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. - [ ] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. - [ ] PASS이면 active task 디렉터리 `agent-task/m-webhook-revision-reconcile/`를 `agent-task/archive/YYYY/MM/m-webhook-revision-reconcile/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. - [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. - [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-webhook-revision-reconcile/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. -- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-local-G05.md`와 `CODE_REVIEW-local-G05.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [x] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-local-G05.md`와 `CODE_REVIEW-local-G05.md`를 작성하고 `complete.log`를 작성하지 않는다. - [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. - [ ] USER_REVIEW가 연결된 SDD/Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. ## 계획 대비 변경 사항 -_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ +계획에 명시된 모든 API 구현을 변경 없이 수행했다. 타입 추가 → DryRunReconcile → ApplyReconcile → 테스트 작성 순서대로 진행했으며, 외부 API, DB 스키마, provider adapter, worker rollout 등은 Milestone 범위를 벗어난다. ## 주요 설계 결정 -_구현 에이전트가 주요 설계 결정 사항을 기록한다._ +- **ReconcileStatus 열거형**: `noop`, `conflict`, `proposal`, `applied`, `duplicate` 5가지 상태로 정의했다. SDD state machine과 일관성을 유지한다. +- **DryRunReconcile**: side effect(이벤트 append, webhook delivery, broadcast)를 전혀 만들지 않는다. expected/current mismatch → conflict, current==target → noop, current!=target → proposal/drift 순서로 판정한다. +- **ApplyReconcile**: 먼저 `DryRunReconcile`을 호출해 conflict면 바로 반환한다. apply는 필수 `IdempotencyKey`와 durable `ProviderDeliveries` store를 모두 요구한다. 둘 중 하나라도 없으면 conflict로 반환하고 side effect를 만들지 않는다. store가 있으면 `RecordOnce` dedupe로 기존 record 확인 후, 첫 apply일 때만 `HandleRevision(ctx, provider, "reconcile:"+IdempotencyKey, revision)` 호출로 webhook/broadcast side effect를 실행하고, duplicate record는 `Status=duplicate`로 반환한다. +- **이중 dedupe 보장**: apply는 `IdempotencyKey` 기반 dedupe와 `HandleRevision` 내부의 `ProviderDeliveries` dedupe 2중 보호를 갖는다. 같은 idempotency key 재호출 시 첫 ApplyReconcile이 기록한 delivery record로 duplicate가 감지되고 HandleRevision은 호출되지 않는다. ## 사용자 리뷰 요청 @@ -115,41 +118,61 @@ _기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직 _구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ -필수 규칙: -- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. -- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. -- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. -- 사용자 리뷰 요청으로 명령을 끝까지 실행하지 못했다면 `사용자 리뷰 요청`에 실행한 명령, 실제 출력, 미실행 명령의 사유를 기록한다. -- mobile/UI hang, timeout, 또는 2분 무진행은 blind retry를 중단하고 focused rerun 명령과 screenshot/window/UI-tree evidence path를 남기며, 불가능하면 정확한 사유를 남긴다. - ### API-1 중간 검증 ```text $ cd services/core && go test ./internal/controlplane -run TestRuntimeReconcileExpectedRevisionConflict -count=1 -(output) +=== RUN TestRuntimeReconcileExpectedRevisionConflict +--- PASS: TestRuntimeReconcileExpectedRevisionConflict (0.00s) +PASS +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.005s ``` ### API-2 중간 검증 ```text $ cd services/core && go test ./internal/controlplane -run TestRuntimeDryRunReconcileReportsDriftConflictProposal -count=1 -(output) +=== RUN TestRuntimeDryRunReconcileReportsDriftConflictProposal +--- PASS: TestRuntimeDryRunReconcileReportsDriftConflictProposal (0.00s) +PASS +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.005s ``` ### API-3 중간 검증 ```text $ cd services/core && go test ./internal/controlplane -run TestRuntimeApplyReconcileIsIdempotent -count=1 -(output) +=== RUN TestRuntimeApplyReconcileIsIdempotent +--- PASS: TestRuntimeApplyReconcileIsIdempotent (0.00s) +PASS +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.005s ``` ### 최종 검증 ```text $ cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent' -count=1 -(output) +=== RUN TestRuntimeReconcileExpectedRevisionConflict +--- PASS: TestRuntimeReconcileExpectedRevisionConflict (0.00s) +=== RUN TestRuntimeDryRunReconcileReportsDriftConflictProposal +--- PASS: TestRuntimeDryRunReconcileReportsDriftConflictProposal (0.00s) +=== RUN TestRuntimeApplyReconcileIsIdempotent +--- PASS: TestRuntimeApplyReconcileIsIdempotent (0.00s) +PASS +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.005s $ cd services/core && go test ./... -(output) +? git.toki-labs.com/toki/gito/services/core/cmd/server [no test files] +? git.toki-labs.com/toki/gito/services/core/cmd/worker [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/config (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.621s +ok git.toki-labs.com/toki/gito/services/core/internal/core (cached) +? git.toki-labs.com/toki/gito/services/core/internal/events [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/gitengine (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/protosocket (cached) +? git.toki-labs.com/toki/gito/services/core/internal/provider [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/provider/forgejo (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/storage (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/worker (cached) $ git diff --check -(output) +(no output — no whitespace errors) ``` --- @@ -157,3 +180,28 @@ $ git diff --check > **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** > If anything is blank, go back and fill it in before saving this file. > Leave review-agent-only sections unchanged. + +## 코드리뷰 결과 + +- 종합 판정: FAIL + +### 차원별 평가 + +| 차원 | 평가 | 근거 | +|------|------|------| +| correctness | Fail | apply 실패 후 재시도에서 idempotency key가 먼저 소비되어 실제 side effect 없이 duplicate로 수렴할 수 있다. | +| completeness | Fail | S10의 repeated apply/idempotent side effect once 보장이 실패/재시도 경로까지 닫히지 않았다. | +| test coverage | Fail | `TestRuntimeApplyReconcileIsIdempotent`는 성공 후 duplicate와 conflict만 검증하고, 첫 apply 실패 후 같은 key 재시도는 검증하지 않는다. | +| API contract | Fail | 실패한 apply가 이후 duplicate 결과를 만들 수 있어 apply result와 side effect 상태가 불일치한다. | +| code quality | Pass | 관련 변경은 좁고 기존 runtime/store 패턴을 따른다. | +| plan deviation | Fail | 계획은 `HandleRevision`의 durable dedupe 재사용을 요구했지만, 별도 apply dedupe record를 side effect 전에 선점해 retry contract가 깨진다. | +| verification trust | Pass | 구현자가 기록한 focused/full test와 `git diff --check`를 재실행해 동일하게 통과함을 확인했다. | +| spec conformance | Fail | SDD S10 `side effect는 한 번만 실행되고 중복은 idempotent 결과로 수렴한다`가 실패 후 재시도 케이스에서 보장되지 않는다. | + +### 발견된 문제 + +- Required: [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1140)에서 `ApplyReconcile`이 `reconcile:` record를 먼저 저장한 뒤 [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1177)의 `HandleRevision`을 호출한다. `HandleRevision`은 webhook/broadcast 실패 시 자기 delivery dedupe만 롤백하지만 [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:714)과 [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:723), 선점된 apply dedupe key는 지우지 않는다. 그래서 첫 apply가 publish 실패로 side effect를 완료하지 못해도 같은 idempotency key 재시도는 [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1158)에서 duplicate로 반환되어 이벤트/브로드캐스트가 영구히 실행되지 않을 수 있다. apply idempotency record를 성공한 apply와 원자적으로 묶거나, 실패/무효 side effect 경로에서 선점 record를 롤백하고, failing broadcaster 또는 webhook 실패 후 같은 key 재시도 테스트를 추가해야 한다. + +### 다음 단계 + +- FAIL follow-up: Required 결함을 수정하는 다음 active `PLAN-local-G05.md`와 `CODE_REVIEW-local-G05.md`를 작성한다. `USER_REVIEW.md` gate는 트리거되지 않는다. diff --git a/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_1.log b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_1.log new file mode 100644 index 0000000..dbd7cca --- /dev/null +++ b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_1.log @@ -0,0 +1,159 @@ + + +# Code Review Reference - REVIEW_API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with linked evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. + +## 개요 + +date=2026-06-19 +task=m-webhook-revision-reconcile, plan=1, tag=REVIEW_API + +## Archive Evidence Snapshot + +- Archived plan: `agent-task/m-webhook-revision-reconcile/plan_local_G05_0.log` +- Archived review: `agent-task/m-webhook-revision-reconcile/code_review_local_G05_0.log` +- Verdict: FAIL +- Required issue: `ApplyReconcile` consumes apply idempotency before `HandleRevision` completes side effects, so a failed first apply can make retry return duplicate without applying. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/provider-change-request-integration/milestones/webhook-revision-reconcile.md` +- Task ids: + - `expected-rev`: expected revision이 맞지 않으면 conflict로 멈춘다. + - `dry-run`: reconcile dry-run 결과로 drift/conflict/proposal을 반환한다. + - `apply`: idempotency key로 중복 적용 없이 change를 반영한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/provider-change-request-integration/webhook-revision-reconcile/SDD.md` +- Acceptance scenarios: + - `S08`: task=`expected-rev`; evidence=`expected revision mismatch test` + - `S09`: task=`dry-run`; evidence=`reconcile dry-run test with drift/conflict/proposal output` + - `S10`: task=`apply`; evidence=`idempotent apply test with repeated event/key and retry after failed publish` +- Completion mode: spec-check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. 리뷰 완료는 판정 append, active 파일 archive, PASS/WARN/FAIL별 다음 상태 작성, archived review checklist 갱신까지 끝난 상태를 의미합니다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_API-1] Apply idempotency retry contract 수정 | [ ] | + +## 구현 체크리스트 + +- [ ] `ApplyReconcile`에서 apply idempotency key가 side effect 성공 전에 영구 소비되지 않도록 수정했다. +- [ ] 실패한 첫 apply 뒤 같은 idempotency key 재시도가 duplicate가 아니라 적용 재시도로 진행되게 했다. +- [ ] `TestRuntimeApplyReconcileRetryAfterPublishFailure`를 추가하고 통과시켰다. +- [ ] 기존 `TestRuntimeApplyReconcileIsIdempotent`의 성공 후 duplicate 보장을 유지했다. +- [ ] 최종 검증 `cd services/core && go test ./...`와 `git diff --check`를 실행했다. +- [ ] CODE_REVIEW-*-G05.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채웠다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G05_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_local_G05_M.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [ ] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [ ] PASS이면 active task 디렉터리 `agent-task/m-webhook-revision-reconcile/`를 `agent-task/archive/YYYY/MM/m-webhook-revision-reconcile/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-webhook-revision-reconcile/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [x] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-local-G05.md`와 `CODE_REVIEW-local-G05.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 연결된 SDD/Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ + +## 주요 설계 결정 + +_구현 에이전트가 주요 설계 결정 사항을 기록한다._ + +## 사용자 리뷰 요청 + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- 실패한 첫 apply가 idempotency key를 영구 소비하지 않는지 확인한다. +- 같은 idempotency key 재시도가 성공 apply로 진행되고, 성공 이후 duplicate는 side effect를 늘리지 않는지 확인한다. +- 기존 expected-rev/dry-run 테스트가 그대로 통과하는지 확인한다. + +## 검증 결과 + +_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ + +### REVIEW_API-1 중간 검증 +```text +$ cd services/core && go test ./internal/controlplane -run TestRuntimeApplyReconcileRetryAfterPublishFailure -count=1 +(output) +``` + +### 최종 검증 +```text +$ cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1 +(output) + +$ cd services/core && go test ./... +(output) + +$ git diff --check +(output) +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 코드리뷰 결과 + +- 종합 판정: FAIL + +### 차원별 평가 + +| 차원 | 평가 | 근거 | +|------|------|------| +| correctness | Fail | 이전 리뷰의 apply idempotency 선점 결함이 그대로 남아 있다. | +| completeness | Fail | `REVIEW_API-1` 구현 항목과 구현 체크리스트가 모두 미완료 상태이며, 후속 계획의 필수 수정이 반영되지 않았다. | +| test coverage | Fail | 필수 테스트 `TestRuntimeApplyReconcileRetryAfterPublishFailure`가 존재하지 않는다. 실제 명령은 `[no tests to run]`로 종료됐다. | +| API contract | Fail | 실패한 첫 apply 뒤 같은 idempotency key 재시도가 duplicate로 막힐 수 있는 API contract 결함이 여전히 존재한다. | +| code quality | Pass | 이번 루프에서 추가된 품질 저하 코드는 확인되지 않았다. | +| plan deviation | Fail | 후속 계획이 요구한 dedupe 흐름 수정과 회귀 테스트가 수행되지 않았다. | +| verification trust | Fail | active review의 검증 결과가 placeholder `(output)` 상태이고, 필수 테스트가 존재하지 않는다. | +| spec conformance | Fail | SDD S10의 repeated apply/idempotent side effect once 증거가 실패 후 재시도까지 확장되지 않았다. | + +### 발견된 문제 + +- Required: [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1140)의 `ApplyReconcile`이 여전히 `reconcile:` record를 side effect 성공 전에 `RecordOnce`로 선점한다. [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1177)의 `HandleRevision`이 실패하면 이 선점 record는 rollback되지 않으므로, 재시도는 [services/core/internal/controlplane/runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1158)에서 duplicate로 멈출 수 있다. 후속 계획대로 별도 선점 record를 제거하고 `HandleRevision`의 `delivery:reconcile:` dedupe 결과를 apply idempotency로 해석하거나, 모든 실패/no-apply 경로에서 선점 record를 rollback해야 한다. +- Required: [services/core/internal/controlplane/runtime_test.go](/config/workspace/gito/services/core/internal/controlplane/runtime_test.go:2068)에 `TestRuntimeApplyReconcileIsIdempotent`만 있고, 계획이 요구한 `TestRuntimeApplyReconcileRetryAfterPublishFailure`가 없다. `cd services/core && go test ./internal/controlplane -run TestRuntimeApplyReconcileRetryAfterPublishFailure -count=1`은 `[no tests to run]`를 출력했다. 첫 apply publish 실패 후 같은 key 재시도가 applied로 성공하고, 그 이후 duplicate가 side effect를 늘리지 않는 회귀 테스트를 추가해야 한다. +- Required: [agent-task/m-webhook-revision-reconcile/CODE_REVIEW-local-G05.md](/config/workspace/gito/agent-task/m-webhook-revision-reconcile/CODE_REVIEW-local-G05.md:53)의 구현 항목과 구현 체크리스트가 미체크이며, `계획 대비 변경 사항`, `주요 설계 결정`, `검증 결과`가 placeholder 상태다. 구현 완료 후 active review의 구현 에이전트 소유 섹션을 실제 변경과 명령 출력으로 채워야 한다. + +### 다음 단계 + +- FAIL follow-up: Required 결함을 수정하는 다음 active `PLAN-local-G05.md`와 `CODE_REVIEW-local-G05.md`를 작성한다. `USER_REVIEW.md` gate는 트리거되지 않는다. diff --git a/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log new file mode 100644 index 0000000..f043033 --- /dev/null +++ b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log @@ -0,0 +1,194 @@ + + +# Code Review Reference - REVIEW_REVIEW_API + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected SDD decision or selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service setup, generic scope conflicts, loop exhaustion, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only the linked SDD/Milestone lock decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. + +## 개요 + +date=2026-06-19 +task=m-webhook-revision-reconcile, plan=2, tag=REVIEW_REVIEW_API + +## Archive Evidence Snapshot + +- Archived plan: `agent-task/m-webhook-revision-reconcile/plan_local_G05_1.log` +- Archived review: `agent-task/m-webhook-revision-reconcile/code_review_local_G05_1.log` +- Previous loop: `agent-task/m-webhook-revision-reconcile/plan_local_G05_0.log`, `agent-task/m-webhook-revision-reconcile/code_review_local_G05_0.log` +- Verdict: FAIL +- Required summary: + - `ApplyReconcile` still records `reconcile:` before `HandleRevision` completes side effects. + - Required test `TestRuntimeApplyReconcileRetryAfterPublishFailure` is missing; `go test ./internal/controlplane -run TestRuntimeApplyReconcileRetryAfterPublishFailure -count=1` returned `[no tests to run]`. + - The active review stub was left unfilled, so verification evidence was not recorded. +- Affected files: `services/core/internal/controlplane/runtime.go`, `services/core/internal/controlplane/runtime_test.go`, `agent-task/m-webhook-revision-reconcile/CODE_REVIEW-local-G05.md` +- Verification evidence: existing focused tests compile, but they do not cover first apply publish failure followed by retry with the same idempotency key. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/provider-change-request-integration/milestones/webhook-revision-reconcile.md` +- Task ids: + - `expected-rev`: expected revision이 맞지 않으면 conflict로 멈춘다. + - `dry-run`: reconcile dry-run 결과로 drift/conflict/proposal을 반환한다. + - `apply`: idempotency key로 중복 적용 없이 change를 반영한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/provider-change-request-integration/webhook-revision-reconcile/SDD.md` +- Acceptance scenarios: + - `S08`: task=`expected-rev`; evidence=`expected revision mismatch test` + - `S09`: task=`dry-run`; evidence=`reconcile dry-run test with drift/conflict/proposal output` + - `S10`: task=`apply`; evidence=`idempotent apply test with repeated event/key and retry after failed publish` +- Completion mode: spec-check-on-pass + +## 이 파일을 읽는 리뷰 에이전트에게 + +> **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 판정 append, active 파일 archive, PASS/WARN/FAIL별 다음 상태 작성, archived review checklist 갱신까지 끝난 상태를 의미합니다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_REVIEW_API-1] Apply idempotency retry contract 수정 | [x] | + +## 구현 체크리스트 + +- [x] `ApplyReconcile`에서 apply idempotency key가 side effect 성공 전에 영구 소비되지 않도록 수정한다. +- [x] 실패한 첫 apply 뒤 같은 idempotency key 재시도가 duplicate가 아니라 적용 재시도로 진행되게 한다. +- [x] `TestRuntimeApplyReconcileRetryAfterPublishFailure`를 추가하고 통과시킨다. +- [x] 기존 `TestRuntimeApplyReconcileIsIdempotent`의 성공 후 duplicate 보장을 유지한다. +- [x] 최종 검증 `cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1`, `cd services/core && go test ./...`, `git diff --check`를 실행한다. +- [x] `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G05_N.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_local_G05_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-webhook-revision-reconcile/`를 `agent-task/archive/YYYY/MM/m-webhook-revision-reconcile/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [x] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/m-webhook-revision-reconcile/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-local-G05.md`와 `CODE_REVIEW-local-G05.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 연결된 SDD/Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +계획에 명시된 해결 방법 ("`ApplyReconcile`의 별도 선점 `RecordOnce`를 제거하고, `HandleRevision`의 기존 durable dedupe 결과를 apply idempotency로 사용")을 그대로 따랐다. 추가 변경 사항은 없다. + +## 주요 설계 결정 + +`ApplyReconcile`에서 별도의 선적 apply dedupe `RecordOnce` 호출을 완전히 제거하고, `HandleRevision(ctx, req.Provider, "reconcile:"+req.IdempotencyKey, revision)`의 기존 delivery dedupe 및 rollback 경로를 apply idempotency로 재사용한다. +- `HandleRevision`이 `RecordOnce`로 dedupe record를 만들고, webhook/broadcast 실패 시 `DeleteDelivery`로 rollback한다. +- 따라서 같은 idempotency key로 재시도하면 `RecordOnce`가 `First=false`를 반환하고 `record.Duplicate=true`가 되어 `ReconcileStatusDuplicate`를 반환한다. +- 실패한 첫 apply는 dedupe record가 롤백되므로 재시도가 duplicate가 아니라 새 적용 시도로서 진행된다. + +## 검증 결과 + +### REVIEW_REVIEW_API-1 중간 검증 +```text +$ cd services/core && go test ./internal/controlplane -run TestRuntimeApplyReconcileRetryAfterPublishFailure -count=1 +=== RUN TestRuntimeApplyReconcileRetryAfterPublishFailure +--- PASS: TestRuntimeApplyReconcileRetryAfterPublishFailure (0.00s) +PASS +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.006s +``` + +### 최종 검증 +```text +$ cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1 +=== RUN TestRuntimeReconcileExpectedRevisionConflict +--- PASS: TestRuntimeReconcileExpectedRevisionConflict (0.00s) +=== RUN TestRuntimeDryRunReconcileReportsDriftConflictProposal +--- PASS: TestRuntimeDryRunReconcileReportsDriftConflictProposal (0.00s) +=== RUN TestRuntimeApplyReconcileIsIdempotent +--- PASS: TestRuntimeApplyReconcileIsIdempotent (0.00s) +=== RUN TestRuntimeApplyReconcileRetryAfterPublishFailure +--- PASS: TestRuntimeApplyReconcileRetryAfterPublishFailure (0.00s) +PASS +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.005s + +$ cd services/core && go test ./... +? git.toki-labs.com/toki/gito/services/core/cmd/server [no test files] +? git.toki-labs.com/toki/gito/services/core/cmd/worker [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/config (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/controlplane 0.731s +ok git.toki-labs.com/toki/gito/services/core/internal/core (cached) +? git.toki-labs.com/toki/gito/services/core/internal/events [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/gitengine (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/protosocket (cached) +? git.toki-labs.com/toki/gito/services/core/internal/provider [no test files] +ok git.toki-labs.com/toki/gito/services/core/internal/provider/forgejo (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/storage (cached) +ok git.toki-labs.com/toki/gito/services/core/internal/worker (cached) + +$ git diff --check +(no output - no whitespace errors) +``` + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- 실패한 첫 apply가 idempotency key를 영구 소비하지 않는지 확인한다. +- 같은 idempotency key 재시도가 성공 apply로 진행되고, 성공 이후 duplicate는 side effect를 늘리지 않는지 확인한다. +- 기존 expected-rev/dry-run 테스트가 그대로 통과하는지 확인한다. +- active review의 구현 에이전트 소유 섹션이 실제 구현과 검증 출력으로 채워졌는지 확인한다. + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## 코드리뷰 결과 + +- 종합 판정: PASS + +### 차원별 평가 + +| 차원 | 평가 | 근거 | +|------|------|------| +| correctness | Pass | `ApplyReconcile`이 별도 선점 `RecordOnce`를 제거하고 `HandleRevision`의 delivery dedupe/rollback 경로를 사용한다. 실패 후 같은 idempotency key 재시도가 적용으로 진행되고, 성공 후 재호출은 duplicate로 수렴한다. | +| completeness | Pass | 후속 계획의 `REVIEW_REVIEW_API-1` 구현, 회귀 테스트, 기존 테스트 유지, 최종 검증 기록이 모두 완료됐다. | +| test coverage | Pass | `TestRuntimeApplyReconcileRetryAfterPublishFailure`가 추가됐고 기존 expected-rev/dry-run/apply idempotency 테스트와 함께 통과했다. | +| API contract | Pass | apply idempotency key가 성공한 apply에 대해서만 duplicate로 수렴하고 실패한 publish는 재시도 가능하다. | +| code quality | Pass | 리뷰 중 주석 표현 drift를 정리했고 debug 출력, dead code, TODO를 확인하지 못했다. | +| plan deviation | Pass | 계획의 권장 경로인 `HandleRevision(ctx, provider, "reconcile:"+IdempotencyKey, revision)` dedupe 재사용을 그대로 구현했다. | +| verification trust | Pass | focused controlplane test, `cd services/core && go test ./...`, `git diff --check`를 리뷰 중 재실행해 모두 통과했다. | +| spec conformance | Pass | S08/S09/S10 evidence가 focused test 묶음으로 확인됐고, 특히 S10의 실패 후 재시도 및 성공 후 duplicate 수렴이 검증됐다. | + +### 발견된 문제 + +없음 + +### 다음 단계 + +- PASS: `complete.log`를 작성하고 task directory를 archive로 이동한다. diff --git a/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/complete.log b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/complete.log new file mode 100644 index 0000000..b22e750 --- /dev/null +++ b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/complete.log @@ -0,0 +1,56 @@ +# Complete - m-webhook-revision-reconcile + +## 완료 일시 + +2026-06-19 + +## 요약 + +Webhook revision reconcile의 expected-rev, dry-run, apply 작업을 3개 리뷰 루프 끝에 PASS로 완료했다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_local_G05_0.log` | `code_review_local_G05_0.log` | FAIL | apply idempotency key를 side effect 전에 선점해 실패 후 재시도가 duplicate로 막힐 수 있었다. | +| `plan_local_G05_1.log` | `code_review_local_G05_1.log` | FAIL | 후속 수정과 `TestRuntimeApplyReconcileRetryAfterPublishFailure`가 아직 반영되지 않았다. | +| `plan_local_G05_2.log` | `code_review_local_G05_2.log` | PASS | 선점 dedupe를 제거하고 `HandleRevision` delivery dedupe/rollback을 apply idempotency로 재사용했다. | + +## 구현/정리 내용 + +- `DryRunReconcile`과 `ApplyReconcile` API를 추가해 expected revision conflict, dry-run drift/proposal, idempotent apply 결과를 반환한다. +- `ApplyReconcile`이 별도 선점 `RecordOnce`를 만들지 않고 `HandleRevision(ctx, provider, "reconcile:"+IdempotencyKey, revision)`의 기존 delivery dedupe 및 rollback 경로를 사용하도록 정리했다. +- `TestRuntimeApplyReconcileRetryAfterPublishFailure`를 추가해 첫 publish 실패 후 같은 idempotency key 재시도가 적용으로 성공하고, 성공 후 재호출은 duplicate로 수렴함을 검증했다. +- 리뷰 중 stale한 주석 표현을 현재 dedupe 설계에 맞게 정리했다. + +## 최종 검증 + +- `cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1` - PASS; four focused controlplane tests passed. +- `cd services/core && go test ./...` - PASS; all core packages passed. +- `git diff --check` - PASS; no whitespace errors. + +## Roadmap Completion + +- Milestone: `agent-roadmap/phase/provider-change-request-integration/milestones/webhook-revision-reconcile.md` +- Completed task ids: + - `expected-rev`: PASS; evidence=`agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log`, `agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log`; verification=`cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1` + - `dry-run`: PASS; evidence=`agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log`, `agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log`; verification=`cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1` + - `apply`: PASS; evidence=`agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log`, `agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log`; verification=`cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1` +- Not completed task ids: 없음 + +## Spec Completion + +- SDD: `agent-roadmap/sdd/provider-change-request-integration/webhook-revision-reconcile/SDD.md` +- Completed scenario ids: + - `S08`: PASS; task=`expected-rev`; evidence=`agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log`, `agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log`; verification=`TestRuntimeReconcileExpectedRevisionConflict` + - `S09`: PASS; task=`dry-run`; evidence=`agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log`, `agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log`; verification=`TestRuntimeDryRunReconcileReportsDriftConflictProposal` + - `S10`: PASS; task=`apply`; evidence=`agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log`, `agent-task/archive/2026/06/m-webhook-revision-reconcile_1/code_review_local_G05_2.log`; verification=`TestRuntimeApplyReconcileIsIdempotent`, `TestRuntimeApplyReconcileRetryAfterPublishFailure` +- Not completed scenario ids: 없음 + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/m-webhook-revision-reconcile/PLAN-local-G05.md b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_0.log similarity index 100% rename from agent-task/m-webhook-revision-reconcile/PLAN-local-G05.md rename to agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_0.log diff --git a/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_1.log b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_1.log new file mode 100644 index 0000000..cc36d1b --- /dev/null +++ b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_1.log @@ -0,0 +1,106 @@ + + +# Plan - REVIEW_API + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-local-G05.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것이 필수 마지막 단계다. 구현 후 active 파일은 그대로 두고 리뷰 준비 상태를 보고한다. 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 막으면 review stub의 `사용자 리뷰 요청`에 정확한 근거를 남기고 멈춘다. 직접 사용자에게 묻거나, 선택지를 채팅으로 제시하거나, `USER_REVIEW.md`, archive log, `complete.log`를 만들지 않는다. + +## Archive Evidence Snapshot + +- Archived plan: `agent-task/m-webhook-revision-reconcile/plan_local_G05_0.log` +- Archived review: `agent-task/m-webhook-revision-reconcile/code_review_local_G05_0.log` +- Verdict: FAIL +- Required issue: `ApplyReconcile`이 `reconcile:` apply dedupe record를 side effect 성공 전에 선점한다. `HandleRevision`이 webhook/broadcast 실패로 error를 반환하면 자신의 delivery dedupe는 rollback하지만 선점 apply dedupe는 남아, 같은 key 재시도가 duplicate로 막힌다. +- Evidence: [runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1140), [runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1177), [runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1158) +- Verification state: 기존 focused/full Go test와 `git diff --check`는 통과했지만, 실패 후 같은 idempotency key 재시도 테스트가 없다. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/provider-change-request-integration/milestones/webhook-revision-reconcile.md` +- Task ids: + - `expected-rev`: expected revision이 맞지 않으면 conflict로 멈춘다. + - `dry-run`: reconcile dry-run 결과로 drift/conflict/proposal을 반환한다. + - `apply`: idempotency key로 중복 적용 없이 change를 반영한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/provider-change-request-integration/webhook-revision-reconcile/SDD.md` +- Acceptance scenarios: + - `S08`: task=`expected-rev`; evidence=`expected revision mismatch test` + - `S09`: task=`dry-run`; evidence=`reconcile dry-run test with drift/conflict/proposal output` + - `S10`: task=`apply`; evidence=`idempotent apply test with repeated event/key and retry after failed publish` +- Completion mode: spec-check-on-pass + +## 배경 + +기존 구현은 happy path duplicate를 막지만, 첫 apply가 `HandleRevision` 내부 publish 단계에서 실패하면 apply idempotency key가 먼저 소비된다. 이 후속 작업은 apply dedupe를 성공한 apply side effect와 일치시키고, 실패 후 재시도가 같은 key로 실제 apply를 완료할 수 있음을 증명한다. + +## 구현 체크리스트 + +- [ ] `ApplyReconcile`에서 apply idempotency key가 side effect 성공 전에 영구 소비되지 않도록 수정한다. +- [ ] `HandleRevision(ctx, provider, "reconcile:"+IdempotencyKey, revision)`의 기존 durable dedupe를 재사용하거나, 별도 apply dedupe record를 유지한다면 모든 `HandleRevision` error 및 no-apply 경로에서 선점 record를 rollback한다. +- [ ] duplicate 결과는 성공한 기존 apply에 대해서만 반환하고, 실패한 첫 apply 뒤 같은 key 재시도는 duplicate가 아니라 적용 재시도로 진행되게 한다. +- [ ] `TestRuntimeApplyReconcileRetryAfterPublishFailure`를 추가해 첫 apply publish 실패 후 같은 idempotency key 재시도가 성공하는지 검증한다. +- [ ] 기존 `TestRuntimeApplyReconcileIsIdempotent`의 성공 후 duplicate 보장을 유지한다. +- [ ] 최종 검증 `cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1`, `cd services/core && go test ./...`, `git diff --check`를 실행한다. +- [ ] `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. + +### [REVIEW_API-1] Apply idempotency retry contract 수정 + +#### 문제 + +[runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1140)의 선점 apply dedupe record가 [runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1177)의 실제 apply보다 먼저 저장된다. 실패 시 `HandleRevision`은 자기 delivery dedupe만 rollback하므로, 같은 key 재시도가 [runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1158)에서 duplicate로 멈출 수 있다. + +#### 해결 방법 + +권장 경로는 `ApplyReconcile`에서 별도 선점 `RecordOnce`를 제거하고, `HandleRevision(ctx, provider, "reconcile:"+IdempotencyKey, revision)`이 이미 제공하는 `delivery:reconcile:` durable dedupe 결과를 apply idempotency로 해석하는 것이다. `EventRecord.Duplicate`가 true이면 `ReconcileStatusDuplicate`를 반환하고, 첫 성공이면 `ReconcileStatusApplied`를 반환한다. 이렇게 하면 `HandleRevision`의 기존 rollback 정책이 실패 후 재시도를 허용한다. + +별도 apply dedupe record를 유지하는 대안도 가능하지만, `HandleRevision` error, unmatched/no-apply, webhook failure, broadcast failure 경로에서 선점 record가 반드시 삭제되어야 한다. 이 경우 테스트는 같은 key 재시도가 성공 apply로 진행됨을 증명해야 한다. + +#### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/controlplane/runtime.go`: `ApplyReconcile` idempotency 처리 수정. +- [ ] `services/core/internal/controlplane/runtime.go`: duplicate 결과가 성공한 기존 apply에만 대응하도록 반환 조건 정리. +- [ ] `services/core/internal/controlplane/runtime_test.go`: 실패 후 재시도 회귀 테스트 추가. + +#### 테스트 작성 + +- 작성: `services/core/internal/controlplane/runtime_test.go`. +- 테스트명: `TestRuntimeApplyReconcileRetryAfterPublishFailure`. +- assertion: + - 같은 store/watch/request를 사용한다. + - 첫 runtime은 `fakeBroadcaster{err: ...}` 또는 동등한 실패 publisher로 `ApplyReconcile`을 실행해 conflict/error result를 받는다. + - 같은 idempotency key로 성공 broadcaster runtime에서 재시도하면 `Status=applied`, `Applied=true`, `Duplicate=false`가 된다. + - 성공 재시도 후 event/envelope/cursor가 적용 결과를 반영한다. + - 이후 같은 key를 한 번 더 호출하면 `Status=duplicate`이고 side effect count가 늘지 않는다. + +#### 중간 검증 + +```bash +cd services/core && go test ./internal/controlplane -run TestRuntimeApplyReconcileRetryAfterPublishFailure -count=1 +``` + +## 의존 관계 및 구현 순서 + +1. `ApplyReconcile` dedupe 흐름을 먼저 수정한다. +2. 실패 후 재시도 테스트를 추가한다. +3. 기존 S08/S09/S10 focused tests와 전체 core tests를 재실행한다. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `services/core/internal/controlplane/runtime.go` | REVIEW_API-1 | +| `services/core/internal/controlplane/runtime_test.go` | REVIEW_API-1 | + +## 최종 검증 + +```bash +cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1 +cd services/core && go test ./... +git diff --check +``` + +모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log new file mode 100644 index 0000000..3aac910 --- /dev/null +++ b/agent-task/archive/2026/06/m-webhook-revision-reconcile_1/plan_local_G05_2.log @@ -0,0 +1,110 @@ + + +# Plan - REVIEW_REVIEW_API + +## 이 파일을 읽는 구현 에이전트에게 + +`CODE_REVIEW-local-G05.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것이 필수 마지막 단계다. 구현 후 active 파일은 그대로 두고 리뷰 준비 상태를 보고한다. 선택된 SDD 결정 또는 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 막으면 review stub의 `사용자 리뷰 요청`에 정확한 근거를 남기고 멈춘다. 직접 사용자에게 묻거나, 선택지를 채팅으로 제시하거나, `USER_REVIEW.md`, archive log, `complete.log`를 만들지 않는다. + +## Archive Evidence Snapshot + +- Archived plan: `agent-task/m-webhook-revision-reconcile/plan_local_G05_1.log` +- Archived review: `agent-task/m-webhook-revision-reconcile/code_review_local_G05_1.log` +- Previous loop: `agent-task/m-webhook-revision-reconcile/plan_local_G05_0.log`, `agent-task/m-webhook-revision-reconcile/code_review_local_G05_0.log` +- Verdict: FAIL +- Required summary: + - `ApplyReconcile` still records `reconcile:` before `HandleRevision` completes side effects. + - Required test `TestRuntimeApplyReconcileRetryAfterPublishFailure` is missing; `go test ./internal/controlplane -run TestRuntimeApplyReconcileRetryAfterPublishFailure -count=1` returned `[no tests to run]`. + - The active review stub was left unfilled, so verification evidence was not recorded. +- Affected files: `services/core/internal/controlplane/runtime.go`, `services/core/internal/controlplane/runtime_test.go`, `agent-task/m-webhook-revision-reconcile/CODE_REVIEW-local-G05.md` +- Verification evidence: existing focused tests compile, but they do not cover first apply publish failure followed by retry with the same idempotency key. + +## Roadmap Targets + +- Milestone: `agent-roadmap/phase/provider-change-request-integration/milestones/webhook-revision-reconcile.md` +- Task ids: + - `expected-rev`: expected revision이 맞지 않으면 conflict로 멈춘다. + - `dry-run`: reconcile dry-run 결과로 drift/conflict/proposal을 반환한다. + - `apply`: idempotency key로 중복 적용 없이 change를 반영한다. +- Completion mode: check-on-pass + +## Spec Targets + +- SDD: `agent-roadmap/sdd/provider-change-request-integration/webhook-revision-reconcile/SDD.md` +- Acceptance scenarios: + - `S08`: task=`expected-rev`; evidence=`expected revision mismatch test` + - `S09`: task=`dry-run`; evidence=`reconcile dry-run test with drift/conflict/proposal output` + - `S10`: task=`apply`; evidence=`idempotent apply test with repeated event/key and retry after failed publish` +- Completion mode: spec-check-on-pass + +## 배경 + +이전 follow-up이 실행되지 않아 같은 Required 결함이 남아 있다. 현재 `ApplyReconcile`은 apply dedupe record를 먼저 저장한 뒤 `HandleRevision`을 호출한다. `HandleRevision`이 webhook 또는 broadcast 단계에서 실패하면 자기 delivery dedupe만 rollback하고, 선점된 apply dedupe는 남아 같은 key 재시도를 duplicate로 막을 수 있다. + +## 구현 체크리스트 + +- [ ] `ApplyReconcile`에서 apply idempotency key가 side effect 성공 전에 영구 소비되지 않도록 수정한다. +- [ ] 실패한 첫 apply 뒤 같은 idempotency key 재시도가 duplicate가 아니라 적용 재시도로 진행되게 한다. +- [ ] `TestRuntimeApplyReconcileRetryAfterPublishFailure`를 추가하고 통과시킨다. +- [ ] 기존 `TestRuntimeApplyReconcileIsIdempotent`의 성공 후 duplicate 보장을 유지한다. +- [ ] 최종 검증 `cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1`, `cd services/core && go test ./...`, `git diff --check`를 실행한다. +- [ ] `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. + +### [REVIEW_REVIEW_API-1] Apply idempotency retry contract 수정 + +#### 문제 + +[runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1140)의 선점 apply dedupe record가 실제 side effect보다 먼저 기록된다. [runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1177)의 `HandleRevision`이 실패하면 선점 record는 지워지지 않고, 재시도는 [runtime.go](/config/workspace/gito/services/core/internal/controlplane/runtime.go:1158)에서 duplicate로 멈출 수 있다. + +#### 해결 방법 + +권장 경로는 `ApplyReconcile`의 별도 선점 `RecordOnce`를 제거하고, `HandleRevision(ctx, provider, "reconcile:"+IdempotencyKey, revision)`의 기존 durable dedupe 결과를 apply idempotency로 사용하는 것이다. 반환된 `EventRecord.Duplicate`가 true이면 `ReconcileStatusDuplicate`, 첫 성공이면 `ReconcileStatusApplied`로 변환한다. 이렇게 하면 `HandleRevision`의 기존 rollback 정책이 실패 후 재시도를 허용한다. + +별도 apply dedupe record를 유지한다면 `HandleRevision` error, unmatched/no-apply, webhook failure, broadcast failure 경로에서 선점 record를 반드시 rollback한다. + +#### 수정 파일 및 체크리스트 + +- [ ] `services/core/internal/controlplane/runtime.go`: `ApplyReconcile` idempotency 처리 수정. +- [ ] `services/core/internal/controlplane/runtime.go`: duplicate 결과가 성공한 기존 apply에만 대응하도록 반환 조건 정리. +- [ ] `services/core/internal/controlplane/runtime_test.go`: 실패 후 재시도 회귀 테스트 추가. + +#### 테스트 작성 + +- 작성: `services/core/internal/controlplane/runtime_test.go`. +- 테스트명: `TestRuntimeApplyReconcileRetryAfterPublishFailure`. +- assertion: + - 같은 store/watch/request를 사용한다. + - 첫 runtime은 `fakeBroadcaster{err: ...}` 또는 동등한 실패 publisher로 `ApplyReconcile`을 실행해 conflict/error result를 받는다. + - 같은 idempotency key로 성공 broadcaster runtime에서 재시도하면 `Status=applied`, `Applied=true`, `Duplicate=false`가 된다. + - 성공 재시도 후 event/envelope/cursor가 적용 결과를 반영한다. + - 이후 같은 key를 한 번 더 호출하면 `Status=duplicate`이고 side effect count가 늘지 않는다. + +#### 중간 검증 + +```bash +cd services/core && go test ./internal/controlplane -run TestRuntimeApplyReconcileRetryAfterPublishFailure -count=1 +``` + +## 의존 관계 및 구현 순서 + +1. `ApplyReconcile` dedupe 흐름을 수정한다. +2. 실패 후 재시도 테스트를 추가한다. +3. 기존 S08/S09/S10 focused tests와 전체 core tests를 재실행한다. +4. active `CODE_REVIEW-local-G05.md`의 구현 에이전트 소유 섹션을 실제 출력으로 채운다. + +## 수정 파일 요약 + +| 파일 | 항목 | +|------|------| +| `services/core/internal/controlplane/runtime.go` | REVIEW_REVIEW_API-1 | +| `services/core/internal/controlplane/runtime_test.go` | REVIEW_REVIEW_API-1 | + +## 최종 검증 + +```bash +cd services/core && go test ./internal/controlplane -run 'TestRuntimeReconcileExpectedRevisionConflict|TestRuntimeDryRunReconcileReportsDriftConflictProposal|TestRuntimeApplyReconcileIsIdempotent|TestRuntimeApplyReconcileRetryAfterPublishFailure' -count=1 +cd services/core && go test ./... +git diff --check +``` + +모든 코드 변경 완료 후 반드시 `CODE_REVIEW-*-G??.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/services/core/internal/controlplane/runtime.go b/services/core/internal/controlplane/runtime.go index b0fe8a7..0b6bbe1 100644 --- a/services/core/internal/controlplane/runtime.go +++ b/services/core/internal/controlplane/runtime.go @@ -72,6 +72,45 @@ type ProviderPollRevision struct { ObservedAt time.Time } +type ReconcileStatus string + +const ( + ReconcileStatusNoop ReconcileStatus = "noop" + ReconcileStatusConflict ReconcileStatus = "conflict" + ReconcileStatusProposal ReconcileStatus = "proposal" + ReconcileStatusApplied ReconcileStatus = "applied" + ReconcileStatusDuplicate ReconcileStatus = "duplicate" +) + +type ReconcileRequest struct { + Provider string + RepoID string + Branch string + ExpectedRevision string + CurrentRevision string + TargetRevision string + ChangedFiles []core.ChangedFile + IdempotencyKey string + ObservedAt time.Time +} + +type ReconcileProposal struct { + Before string `json:"before"` + After string `json:"after"` + Changed []core.ChangedFile `json:"changed,omitempty"` +} + +type ReconcileResult struct { + Status ReconcileStatus `json:"status"` + Conflict bool `json:"conflict,omitempty"` + Applied bool `json:"applied,omitempty"` + Duplicate bool `json:"duplicate,omitempty"` + Drift bool `json:"drift,omitempty"` + Reason string `json:"reason,omitempty"` + Proposal *ReconcileProposal `json:"proposal,omitempty"` + IdempotencyKey string `json:"idempotency_key,omitempty"` +} + type WebhookSubscription struct { ID string Name string @@ -1001,6 +1040,153 @@ func NewOutboxBroadcaster(store storage.OperationEventStore) EventBroadcaster { return &outboxBroadcaster{store: store} } +// DryRunReconcile validates the reconcile request and returns a proposal without +// applying any changes. It does not append events, create deliveries, or broadcast. +func (r *Runtime) DryRunReconcile(ctx context.Context, req ReconcileRequest) ReconcileResult { + req.Provider = strings.TrimSpace(req.Provider) + if req.Provider == "" { + req.Provider = "forgejo" + } + req.RepoID = strings.TrimSpace(req.RepoID) + req.Branch = strings.TrimSpace(req.Branch) + if req.RepoID == "" || req.Branch == "" { + return ReconcileResult{ + Status: ReconcileStatusConflict, + Conflict: true, + Reason: "repo_id and branch are required", + } + } + + // Check expected revision mismatch first + if req.ExpectedRevision != "" && req.CurrentRevision != "" && req.ExpectedRevision != req.CurrentRevision { + return ReconcileResult{ + Status: ReconcileStatusConflict, + Conflict: true, + Applied: false, + Reason: fmt.Sprintf("expected revision %q does not match current revision %q", req.ExpectedRevision, req.CurrentRevision), + } + } + + // No drift when current matches target + if req.CurrentRevision == req.TargetRevision { + return ReconcileResult{ + Status: ReconcileStatusNoop, + Drift: false, + Applied: false, + Reason: "current revision already matches target", + } + } + + // Drift proposal: current differs from target + return ReconcileResult{ + Status: ReconcileStatusProposal, + Drift: true, + Applied: false, + Proposal: &ReconcileProposal{ + Before: req.CurrentRevision, + After: req.TargetRevision, + Changed: req.ChangedFiles, + }, + Reason: "drift detected: current differs from target", + } +} + +// ApplyReconcile validates the reconcile request, runs dry-run, and applies the +// change using HandleRevision with an idempotency key. Side effects (webhook +// delivery, broadcast) happen only on first apply; duplicate keys return +// ReconcileStatusDuplicate without additional side effects. +// Apply idempotency is delegated to HandleRevision's delivery dedupe path, so a +// failure during HandleRevision rolls back the dedupe record and allows retry. +// IdempotencyKey and durable ProviderDeliveries store are required; missing +// either returns conflict without side effects. +func (r *Runtime) ApplyReconcile(ctx context.Context, req ReconcileRequest) ReconcileResult { + req.Provider = strings.TrimSpace(req.Provider) + if req.Provider == "" { + req.Provider = "forgejo" + } + req.RepoID = strings.TrimSpace(req.RepoID) + req.Branch = strings.TrimSpace(req.Branch) + if req.RepoID == "" || req.Branch == "" { + return ReconcileResult{ + Status: ReconcileStatusConflict, + Conflict: true, + Reason: "repo_id and branch are required", + } + } + + // Dry-run first: conflict blocks apply + dryResult := r.DryRunReconcile(ctx, req) + if dryResult.Conflict { + return dryResult + } + + // Idempotency key is required for apply + if req.IdempotencyKey == "" { + return ReconcileResult{ + Status: ReconcileStatusConflict, + Conflict: true, + Reason: "idempotency_key is required for apply", + } + } + + // Durable dedupe store is required for apply + if r.store == nil || r.store.ProviderDeliveries() == nil { + return ReconcileResult{ + Status: ReconcileStatusConflict, + Conflict: true, + Reason: "durable ProviderDeliveries store is required for apply", + } + } + + // Apply via HandleRevision. The existing delivery dedupe path inside + // HandleRevision acts as both the delivery dedupe and the apply idempotency + // key. When HandleRevision fails it rolls back its delivery dedupe record, + // so the same idempotency key can be retried successfully. + revision := core.RevisionEvent{ + RepoID: req.RepoID, + Branch: req.Branch, + Before: req.CurrentRevision, + After: req.TargetRevision, + ChangedFiles: req.ChangedFiles, + ObservedAt: req.ObservedAt, + } + record, matched, err := r.HandleRevision(ctx, req.Provider, "reconcile:"+req.IdempotencyKey, revision) + if err != nil { + return ReconcileResult{ + Status: ReconcileStatusConflict, + Conflict: true, + Reason: fmt.Sprintf("handle revision: %v", err), + } + } + if record.Duplicate { + // HandleRevision returned an idempotent duplicate result. This means + // the previous invocation with the same delivery id already completed + // side effects (webhook delivery, broadcast, cursor upsert). + return ReconcileResult{ + Status: ReconcileStatusDuplicate, + Duplicate: true, + Applied: false, + IdempotencyKey: req.IdempotencyKey, + Reason: "duplicate reconcile apply by idempotency key", + } + } + if !matched { + return ReconcileResult{ + Status: ReconcileStatusApplied, + Applied: true, + IdempotencyKey: req.IdempotencyKey, + Reason: "applied but event did not match any watch", + } + } + return ReconcileResult{ + Status: ReconcileStatusApplied, + Applied: true, + Duplicate: false, + IdempotencyKey: req.IdempotencyKey, + Reason: "reconcile apply succeeded", + } +} + func (o *outboxBroadcaster) BroadcastEnvelope(ctx context.Context, env protosocket.Envelope) error { if env.Channel != "event" || env.Action != "branch.updated" { return nil diff --git a/services/core/internal/controlplane/runtime_test.go b/services/core/internal/controlplane/runtime_test.go index 85da25e..c1b60c9 100644 --- a/services/core/internal/controlplane/runtime_test.go +++ b/services/core/internal/controlplane/runtime_test.go @@ -1911,6 +1911,240 @@ func TestRuntime_WebhookDeliveryFailureAllowsRetry(t *testing.T) { // TestWebhookDeliveryBranchUpdated is an alias for TestRuntime_WebhookDeliveryBranchUpdated // to match the SDD test evidence command: go test -run TestWebhookDeliveryBranchUpdated +// --- Reconcile Tests --- + +func TestRuntimeReconcileExpectedRevisionConflict(t *testing.T) { + store := newFakeStore() + broadcaster := &fakeBroadcaster{} + runtime := NewRuntimeWithStore(broadcaster, store) + + // Register branch watch so events could match if emitted + _, err := runtime.RegisterBranchWatch("nomadcode", "develop", "forgejo") + if err != nil { + t.Fatalf("register watch: %v", err) + } + + // Dry-run with expected/current mismatch must return conflict + req := ReconcileRequest{ + Provider: "forgejo", + RepoID: "nomadcode", + Branch: "develop", + ExpectedRevision: "rev-expected", + CurrentRevision: "rev-current", + TargetRevision: "rev-target", + } + result := runtime.DryRunReconcile(context.Background(), req) + + if !result.Conflict { + t.Fatal("expected conflict for expected/current revision mismatch") + } + if result.Status != ReconcileStatusConflict { + t.Fatalf("status: got %q want %q", result.Status, ReconcileStatusConflict) + } + if result.Applied { + t.Fatal("expected applied=false for conflict") + } + if result.Reason == "" { + t.Fatal("expected reason to be set") + } + + // Events and broadcaster must not be affected by dry-run + if len(runtime.ListEvents()) != 0 { + t.Fatalf("expected 0 events after dry-run, got %d", len(runtime.ListEvents())) + } + if len(broadcaster.envelopes) != 0 { + t.Fatalf("expected 0 envelopes after dry-run, got %d", len(broadcaster.envelopes)) + } + + // Apply with expected/current mismatch must also return conflict without side effects + applyReq := req + applyReq.IdempotencyKey = "conflict-apply-key" + applyResult := runtime.ApplyReconcile(context.Background(), applyReq) + if !applyResult.Conflict { + t.Fatal("expected conflict for apply with expected/current mismatch") + } + if applyResult.Applied { + t.Fatal("expected applied=false for conflict apply") + } + if len(runtime.ListEvents()) != 0 { + t.Fatal("apply conflict must not create events") + } + if len(broadcaster.envelopes) != 0 { + t.Fatal("apply conflict must not broadcast") + } +} + +func TestRuntimeDryRunReconcileReportsDriftConflictProposal(t *testing.T) { + store := newFakeStore() + broadcaster := &fakeBroadcaster{} + runtime := NewRuntimeWithStore(broadcaster, store) + + // Register watch + _, err := runtime.RegisterBranchWatch("nomadcode", "develop", "forgejo") + if err != nil { + t.Fatalf("register watch: %v", err) + } + + // Conflict case: expected != current + conflictReq := ReconcileRequest{ + Provider: "forgejo", + RepoID: "nomadcode", + Branch: "develop", + ExpectedRevision: "rev-a", + CurrentRevision: "rev-b", + TargetRevision: "rev-c", + } + conflictResult := runtime.DryRunReconcile(context.Background(), conflictReq) + if !conflictResult.Conflict { + t.Fatal("expected conflict status") + } + if conflictResult.Status != ReconcileStatusConflict { + t.Fatalf("conflict status: got %q", conflictResult.Status) + } + + // Noop case: current == target + noopReq := ReconcileRequest{ + Provider: "forgejo", + RepoID: "nomadcode", + Branch: "develop", + CurrentRevision: "rev-same", + TargetRevision: "rev-same", + } + noopResult := runtime.DryRunReconcile(context.Background(), noopReq) + if noopResult.Status != ReconcileStatusNoop { + t.Fatalf("noop status: got %q want %q", noopResult.Status, ReconcileStatusNoop) + } + if noopResult.Drift { + t.Fatal("expected drift=false for noop") + } + if noopResult.Applied { + t.Fatal("expected applied=false for dry-run noop") + } + + // Proposal/drift case: current != target, no expected mismatch + proposalReq := ReconcileRequest{ + Provider: "forgejo", + RepoID: "nomadcode", + Branch: "develop", + CurrentRevision: "rev-current", + TargetRevision: "rev-target", + ChangedFiles: []core.ChangedFile{ + {Path: "README.md", ChangeType: "modified"}, + }, + } + proposalResult := runtime.DryRunReconcile(context.Background(), proposalReq) + if proposalResult.Status != ReconcileStatusProposal { + t.Fatalf("proposal status: got %q want %q", proposalResult.Status, ReconcileStatusProposal) + } + if !proposalResult.Drift { + t.Fatal("expected drift=true for proposal") + } + if proposalResult.Applied { + t.Fatal("expected applied=false for dry-run proposal") + } + if proposalResult.Proposal == nil { + t.Fatal("expected proposal to be set") + } else { + if proposalResult.Proposal.Before != "rev-current" { + t.Errorf("proposal before: got %q want %q", proposalResult.Proposal.Before, "rev-current") + } + if proposalResult.Proposal.After != "rev-target" { + t.Errorf("proposal after: got %q want %q", proposalResult.Proposal.After, "rev-target") + } + if len(proposalResult.Proposal.Changed) != 1 { + t.Fatalf("expected 1 changed file in proposal, got %d", len(proposalResult.Proposal.Changed)) + } + } + + // Dry-run must never create events or broadcasts + if len(runtime.ListEvents()) != 0 { + t.Fatalf("expected 0 events after dry-run, got %d", len(runtime.ListEvents())) + } + if len(broadcaster.envelopes) != 0 { + t.Fatalf("expected 0 envelopes after dry-run, got %d", len(broadcaster.envelopes)) + } +} + +func TestRuntimeApplyReconcileIsIdempotent(t *testing.T) { + store := newFakeStore() + broadcaster := &fakeBroadcaster{} + runtime := NewRuntimeWithStore(broadcaster, store) + + // Register watch + _, err := runtime.RegisterBranchWatch("nomadcode", "develop", "forgejo") + if err != nil { + t.Fatalf("register watch: %v", err) + } + + req := ReconcileRequest{ + Provider: "forgejo", + RepoID: "nomadcode", + Branch: "develop", + CurrentRevision: "rev-before", + TargetRevision: "rev-after", + IdempotencyKey: "idempotent-key-1", + ChangedFiles: []core.ChangedFile{ + {Path: "README.md", ChangeType: "modified"}, + }, + ObservedAt: time.Now().UTC(), + } + + // First apply: should succeed and create 1 event + 1 envelope + firstResult := runtime.ApplyReconcile(context.Background(), req) + if firstResult.Status != ReconcileStatusApplied { + t.Fatalf("first apply status: got %q want %q", firstResult.Status, ReconcileStatusApplied) + } + if !firstResult.Applied { + t.Fatal("expected applied=true on first apply") + } + if firstResult.Duplicate { + t.Fatal("expected duplicate=false on first apply") + } + if len(runtime.ListEvents()) != 1 { + t.Fatalf("expected 1 event after first apply, got %d", len(runtime.ListEvents())) + } + if len(broadcaster.envelopes) != 1 { + t.Fatalf("expected 1 envelope after first apply, got %d", len(broadcaster.envelopes)) + } + + // Second apply with same idempotency key: should return duplicate without side effects + secondResult := runtime.ApplyReconcile(context.Background(), req) + if secondResult.Status != ReconcileStatusDuplicate { + t.Fatalf("second apply status: got %q want %q", secondResult.Status, ReconcileStatusDuplicate) + } + if secondResult.Applied { + t.Fatal("expected applied=false on duplicate apply") + } + if !secondResult.Duplicate { + t.Fatal("expected duplicate=true on second apply") + } + + // Side effects must not increase on duplicate + if len(runtime.ListEvents()) != 1 { + t.Fatalf("expected 1 event after duplicate apply, got %d", len(runtime.ListEvents())) + } + if len(broadcaster.envelopes) != 1 { + t.Fatalf("expected 1 envelope after duplicate apply, got %d", len(broadcaster.envelopes)) + } + + // Expected/current mismatch apply must return conflict without side effects + conflictReq := req + conflictReq.ExpectedRevision = "rev-mismatch" + conflictResult := runtime.ApplyReconcile(context.Background(), conflictReq) + if !conflictResult.Conflict { + t.Fatal("expected conflict for apply with expected mismatch") + } + if conflictResult.Applied { + t.Fatal("expected applied=false for conflict apply") + } + if len(runtime.ListEvents()) != 1 { + t.Fatal("conflict apply must not create additional events") + } + if len(broadcaster.envelopes) != 1 { + t.Fatal("conflict apply must not broadcast additional envelopes") + } +} + func TestWebhookDeliveryBranchUpdated(t *testing.T) { TestRuntime_WebhookDeliveryBranchUpdated(t) } @@ -2156,9 +2390,84 @@ func TestWebhookDeliveryDoesNotDispatchUnrelatedPendingDelivery(t *testing.T) { } } -// TestWebhookDeliveryRecordOutcomeError verifies that when RecordOutcome fails (e.g. due to -// a fake store returning an error), the error is propagated up so the caller knows the outcome -// was not persisted. +// TestRuntimeApplyReconcileRetryAfterPublishFailure verifies that when the first +// ApplyReconcile fails during webhook/broadcast side effects, the apply idempotency +// key is NOT consumed, and a retry with the same key succeeds. +func TestRuntimeApplyReconcileRetryAfterPublishFailure(t *testing.T) { + store := newFakeStore() + + // Register branch watch so the reconcile event matches + _, err := store.watches.UpsertBranchWatch(context.Background(), core.BranchWatch{ + ID: "watch-nomadcode-develop", + Provider: "forgejo", + RepoID: "nomadcode", + Branch: "develop", + }) + if err != nil { + t.Fatalf("upsert branch watch: %v", err) + } + + // 1. First apply with failing broadcaster -> conflict + failingBroadcaster := &fakeBroadcaster{err: fmt.Errorf("broadcast failed")} + rFail := NewRuntimeWithStore(failingBroadcaster, store) + + applyReq := ReconcileRequest{ + Provider: "forgejo", + RepoID: "nomadcode", + Branch: "develop", + CurrentRevision: "rev-before", + TargetRevision: "rev-after", + IdempotencyKey: "retry-key-1", + ChangedFiles: []core.ChangedFile{ + {Path: "README.md", ChangeType: "modified"}, + }, + ObservedAt: time.Now().UTC(), + } + + firstResult := rFail.ApplyReconcile(context.Background(), applyReq) + if firstResult.Status != ReconcileStatusConflict { + t.Fatalf("first apply status: got %q want %q", firstResult.Status, ReconcileStatusConflict) + } + if !firstResult.Conflict { + t.Fatal("first apply: expected conflict=true") + } + + // 2. Second apply with succeeding broadcaster -> applied + successBroadcaster := &fakeBroadcaster{} + rSuccess := NewRuntimeWithStore(successBroadcaster, store) + + secondResult := rSuccess.ApplyReconcile(context.Background(), applyReq) + if secondResult.Status != ReconcileStatusApplied { + t.Fatalf("second apply status: got %q want %q", secondResult.Status, ReconcileStatusApplied) + } + if !secondResult.Applied { + t.Fatal("second apply: expected applied=true") + } + if secondResult.Duplicate { + t.Fatal("second apply: expected duplicate=false") + } + + // 3. Third apply with same key -> duplicate (side effects not repeated) + thirdResult := rSuccess.ApplyReconcile(context.Background(), applyReq) + if thirdResult.Status != ReconcileStatusDuplicate { + t.Fatalf("third apply status: got %q want %q", thirdResult.Status, ReconcileStatusDuplicate) + } + if thirdResult.Applied { + t.Fatal("third apply: expected applied=false") + } + if !thirdResult.Duplicate { + t.Fatal("third apply: expected duplicate=true") + } + + // 4. Events/envelopes count: successful retry creates 1 event + 1 envelope; duplicate creates nothing. + if len(rSuccess.ListEvents()) != 1 { + t.Fatalf("expected 1 event total, got %d", len(rSuccess.ListEvents())) + } + if len(successBroadcaster.envelopes) != 1 { + t.Fatalf("expected 1 envelope total, got %d", len(successBroadcaster.envelopes)) + } +} + func TestWebhookDeliveryRecordOutcomeError(t *testing.T) { // Create a runtime with a store that has a webhook delivery store which returns an error on RecordOutcome. store := newFakeStore()