<!-- task=m-webhook-revision-reconcile/02+01_retry_delivery_store plan=2 tag=REVIEW_REVIEW_API -->

# Plan - REVIEW_REVIEW_API

## 이 파일을 읽는 구현 에이전트에게

`CODE_REVIEW-local-G04.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우는 것이 필수다. 구현 후 검증을 실행하고 실제 stdout/stderr를 붙인 뒤 active 파일을 그대로 두고 리뷰 준비를 보고한다. 최종 판정, 로그 rename, `complete.log`, archive 이동은 code-review-skill 전용이다. 선택된 SDD 결정 또는 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 막으면 review stub의 `사용자 리뷰 요청` 섹션에 연결 대상, 근거, 실행 명령, 재개 조건을 기록하고 멈춘다. 구현 중 사용자에게 직접 묻거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 환경/secret/service blocker, 일반 범위 변경, 후속 에이전트가 닫을 수 있는 검증 공백은 사용자 리뷰 요청이 아니다.

## 배경

이전 follow-up은 immediate dispatch를 id-scoped `StartDelivery`로 전환하고 `RecordOutcome` 오류를 전파해 런타임 Required를 대부분 해소했다. 그러나 PostgreSQL integration evidence가 되어야 할 `TestPostgresWebhookDeliveryStoreRecordsRetryState` 안의 `StartDelivery` assertion이 due 조건을 거꾸로 구성했다. 현재 테스트는 `next_attempt_at=now`인 delivery에 `time.Now().Add(time.Hour)`를 넘기면서 not-yet-due라고 기대하므로, 실제 DB 환경에서는 `next_attempt_at <= now` 조건을 만족해 `ok=true`가 될 수 있다.

## 사용자 리뷰 요청 흐름

구현 중 직접 사용자에게 묻지 않는다. 선택된 SDD 결정 또는 Milestone lock 결정만 active review stub의 `사용자 리뷰 요청` 섹션에 기록하고, code-review가 검증해 실제 `USER_REVIEW.md` 작성 여부를 결정한다.

## Archive Evidence Snapshot

- Current archived plan: `agent-task/m-webhook-revision-reconcile/02+01_retry_delivery_store/plan_local_G05_1.log`
- Current archived review: `agent-task/m-webhook-revision-reconcile/02+01_retry_delivery_store/code_review_local_G05_1.log`
- Verdict: FAIL
- Required summary:
  - `services/core/internal/storage/postgres_test.go:1324` calls `StartDelivery(ctx, delB.ID, time.Now().Add(time.Hour))` while expecting a not-yet-due result. Because `StartDelivery` uses `next_attempt_at <= $2` in `services/core/internal/storage/postgres.go:1333`, the delivery is due in that call and the assertion is invalid.
- Suggested/Nit summary: 없음
- Affected files: `services/core/internal/storage/postgres_test.go`.
- Verification evidence: controlplane targeted tests, retry/Postgres targeted tests, `go test ./...`, and `git diff --check` passed locally, but PostgreSQL integration skipped because `GITO_TEST_DATABASE_URL` was unset.
- Roadmap/spec carryover: Roadmap task `retry`, SDD scenario `S04`.

## Roadmap Targets

- Milestone: `agent-roadmap/phase/provider-change-request-integration/milestones/webhook-revision-reconcile.md`
- Task ids:
  - `retry`: delivery idempotency, retry/backoff, failed delivery 상태를 PostgreSQL 기준으로 기록한다.
- Completion mode: check-on-pass

## Spec Targets

- SDD: `agent-roadmap/sdd/provider-change-request-integration/webhook-revision-reconcile/SDD.md`
- Acceptance scenarios:
  - `S04`: task=`retry`; evidence=`delivery retry/backoff unit or integration test with PostgreSQL store`
- Completion mode: spec-check-on-pass

## 분석 결과

### 읽은 파일

- `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-test/local/rules.md`
- `agent-test/local/core-smoke.md`
- `agent-ops/rules/project/domain/core/rules.md`
- `services/core/internal/storage/postgres.go`
- `services/core/internal/storage/postgres_test.go`
- `agent-task/m-webhook-revision-reconcile/02+01_retry_delivery_store/code_review_local_G05_1.log`

### 테스트 환경 규칙

- test_env: `local`.
- 적용 규칙: `agent-test/local/rules.md`, `agent-test/local/core-smoke.md`.
- core behavior change 또는 storage behavior evidence 변경 후 필수 검증: `cd services/core && go test ./...`.
- PostgreSQL-dependent test는 `GITO_TEST_DATABASE_URL`이 없으면 skip 가능하지만, skip 출력과 residual risk를 review stub에 기록한다.

### 범위 결정 근거

- 이번 follow-up은 `StartDelivery` 구현 자체가 아니라 그 Postgres evidence test를 고치는 작업이다.
- 런타임 dispatch path, provider callback payload, migration schema, storage interface는 변경하지 않는다.
- 테스트가 실제 due/not-due 조건을 명확히 만들기 위해 필요한 범위에서만 fixture 값을 조정한다.

### 빌드 등급

- build/review: `local-G04`. 단일 Postgres integration test의 assertion/fixture 정정이며, 로컬 full core smoke와 targeted storage test로 검증 가능하다.

## 의존 관계 및 구현 순서

1. `TestPostgresWebhookDeliveryStoreRecordsRetryState`의 `StartDelivery` assertion 구간을 읽고 현재 fixture 흐름을 정리한다.
2. not-yet-due delivery는 `NextAttemptAt`을 명시적으로 미래로 설정하고, 그보다 이른 `now`를 넘겨 `ok=false`와 상태 유지(`pending` 또는 `retryable`)를 검증한다.
3. due id-scoped transition은 별도 due delivery를 대상으로 `StartDelivery(ctx, dueID, now)`를 호출하고, 같은 시각에 due/pending인 다른 delivery가 함께 전이되지 않는지 확인한다.
4. `postgres_test.go:1335` 이후의 실험 중 남은 주석을 제거하고 실제 assertion으로 대체한다.
5. targeted Postgres test와 core smoke를 실행하고 결과를 `CODE_REVIEW-local-G04.md`에 기록한다.

## 구현 체크리스트

- [ ] `services/core/internal/storage/postgres_test.go`의 not-yet-due `StartDelivery` fixture를 실제 미래 `NextAttemptAt` 기준으로 고친다.
- [ ] `StartDelivery(ctx, id, now)`가 target due delivery만 `sending`으로 전이하고 unrelated due/pending delivery를 건드리지 않는 assertion을 추가하거나 정리한다.
- [ ] `postgres_test.go`의 실험 중 남은 설명성 주석을 제거하고 테스트 의도를 assertion 이름/fixture로 드러낸다.
- [ ] `cd services/core && go test -run TestPostgresWebhookDeliveryStoreRecordsRetryState ./internal/storage -v -count=1`를 실행한다.
- [ ] `cd services/core && go test -run 'TestWebhookDeliveryRetry|TestPostgresWebhookDelivery' ./internal/... -v -count=1`를 실행한다.
- [ ] `cd services/core && go test ./...`를 실행한다.
- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다.

## [REVIEW_REVIEW_API-1] Fix Postgres StartDelivery Evidence

### 문제

`services/core/internal/storage/postgres_test.go:1324`가 not-yet-due delivery를 검증한다는 설명과 달리 `time.Now().Add(time.Hour)`를 `StartDelivery`의 `now`로 전달한다. enqueue 기본값은 `NextAttemptAt`을 현재 시각으로 저장하므로, 한 시간 뒤 `now` 기준에서는 해당 delivery가 due 상태다. 이 테스트는 실제 PostgreSQL 환경에서 실패하거나, 실행되지 않으면 S04 evidence 공백을 숨긴다.

### 해결 방법

테스트 fixture를 명시적인 기준 시각으로 고정한다. 예를 들어 `baseNow := time.Now().UTC()`를 잡고, not-yet-due delivery는 `NextAttemptAt: baseNow.Add(time.Hour)`로 enqueue한다. 이후 `StartDelivery(ctx, notDueID, baseNow)`가 `(zero, false, nil)`을 반환하고 상태가 바뀌지 않는지 확인한다. 별도 due delivery는 `NextAttemptAt: baseNow.Add(-time.Minute)` 또는 기본 due 값으로 만들고 `StartDelivery(ctx, dueID, baseNow)`가 그 id만 `sending`으로 전이하는지 확인한다.

### 수정 파일 및 체크리스트

- [ ] `services/core/internal/storage/postgres_test.go`: not-yet-due fixture의 `NextAttemptAt`을 미래로 명시한다.
- [ ] `services/core/internal/storage/postgres_test.go`: due fixture와 unrelated fixture를 분리해 id-scoped transition을 검증한다.
- [ ] `services/core/internal/storage/postgres_test.go`: stale exploratory comments를 제거한다.

### 중간 검증

```bash
cd services/core && go test -run TestPostgresWebhookDeliveryStoreRecordsRetryState ./internal/storage -v -count=1
```

Expected: `GITO_TEST_DATABASE_URL`이 있으면 실제 assertion이 실행되어 통과한다. 환경변수가 없으면 skip 출력과 residual risk를 review stub에 기록한다.
