diff --git a/.proofline/lines/line-0004/evidence/first-qc-review.md b/.proofline/lines/line-0004/evidence/first-qc-review.md new file mode 100644 index 0000000..5aadc93 --- /dev/null +++ b/.proofline/lines/line-0004/evidence/first-qc-review.md @@ -0,0 +1,102 @@ +# line-0004 1차 구현 QC 리뷰 결과 + +- **대상 이슈:** `fourblocks2020/portfolio-hub#27` +- **대상 Line:** `line-0004` +- **검토 브랜치:** `line/line-0004-implementation` +- **검토 커밋:** `e2102b6f2b855b7679ab67818202ef706d9848fb` +- **범위 권위:** confirmed `dcy-0004` 및 승인된 GitHub Issue #27 본문 +- **검토 시각:** `2026-08-15T04:10:35Z` (`2026-08-15 13:10:35 KST`) +- **검토 방식:** 팀장 직접 재실행 + 별도 read-only 독립 QC reviewer +- **판정:** **FAIL — Blocker 1건** +- **GitHub QC 댓글:** + +## 1. 판정 요약 + +`POST /v1/portfolios`의 `managed_by_daemon_id` persistence와 생성/PATCH 배정 검증의 주요 경로는 구현되었다. 그러나 승인된 Discovery가 요구한 daemon owner 재할당과 portfolio assignment 사이의 동일 소유자 불변식이 유지되지 않는다. + +따라서 이번 리뷰는 QC PASS가 아니며 구현 완료·검증 완료, main 반영, push, PR, 배포 또는 release를 승인하지 않는다. + +## 2. 통과한 검증 + +팀장 측에서 실제로 재실행한 결과: + +| 검증 | 결과 | +| --- | --- | +| 집중 daemon assignment API 및 CRUD | `37 passed` | +| 전체 단위·회귀 스위트 | `625 passed, 11 skipped` | +| migration head 적용 disposable PostgreSQL integration | `4 passed` | +| 변경 핵심 파일 Ruff | `All checks passed!` | +| Python `compileall` | PASS | +| `git diff --check` | PASS | +| `proofline validate` | PASS | +| `trex-web` 변경 여부 | 변경 없음 — Out of Scope 준수 | + +별도 read-only QC reviewer도 집중 테스트, 전체 회귀, 실제 PostgreSQL 검증을 재실행했으며 동일 blocker를 독립적으로 재현하고 `FAIL`로 판정했다. + +### 승인 후보 기준별 요약 + +`line-0004`에는 canonical approved REQ와 연결 active AC가 없으므로 `dcy-0004.md`의 승인 후보 12개를 기준으로 검토했다. + +- POST 성공 응답의 daemon ID: PASS +- POST 후 조회 persistence: PASS +- 미선택 생성 시 null 유지: PASS +- malformed UUID `422` 및 무변경: PASS +- unknown/foreign daemon `404` 및 무변경: PASS +- inactive daemon `409` 및 무변경: PASS +- admin target-owner 기준: 구현 PASS, PATCH 전용 직접 테스트 보강 필요 +- PATCH 비-null 지정·재지정 및 오류 무변경: 주요 경로 PASS +- validation 실패 시 partial create/update 금지: PASS +- assignment와 daemon deactivation 직렬화: PASS +- assignment 후 daemon deactivation 시 reference 보존: 구현 계약 확인 +- 전체 회귀 및 Out of Scope: PASS + +## 3. Blocker — daemon owner transfer 후 동일 소유자 불변식 붕괴 + +승인된 Discovery는 daemon assignment와 daemon 소유자 재할당이 경합해도 최종 관계가 다음 규칙을 위반하지 않아야 한다고 명시한다. + +```text +daemon.owner_user_id == portfolio.owner_user_id +``` + +현재 assignment 구현은 daemon row를 `FOR SHARE`로 잠그지만, `app/repositories/postgres.py`의 `update_daemon_owner()`는 이미 해당 daemon을 참조하는 portfolio를 검사하거나 owner transfer를 거부하지 않는다. + +migration head가 적용된 disposable PostgreSQL에서 다음 상태를 실제로 재현했다. + +```text +before portfolio_owner=owner-a daemon_owner=owner-a +after portfolio_owner=owner-a daemon_owner=owner-b +assignment= +invariant_same_owner=False +``` + +즉 다음 실행 순서가 허용된다. + +1. owner A의 portfolio에 owner A의 daemon assignment가 commit된다. +2. 같은 daemon의 owner transfer가 owner B로 commit된다. +3. portfolio는 owner A이고 daemon은 owner B인 foreign assignment가 영구 저장된다. + +현재 추가된 concurrency test는 daemon deactivation만 다루며 owner transfer 순차·경합 경로를 다루지 않는다. + +## 4. 수정 방향 + +- 참조 중인 daemon의 owner transfer를 원자적으로 거부한다. +- assignment와 owner transfer가 같은 daemon row-lock 경계와 일관된 lock order를 사용하도록 한다. +- 자동 assignment 해제, portfolio owner 이전, 다른 daemon 재배정, backfill은 수행하지 않는다. 이 항목들은 승인 범위 밖이다. +- 참조 충돌을 전용 domain error로 표현하고 admin daemon owner 변경 API에서 `409 Conflict`로 매핑한다. +- 충돌 시 daemon owner, portfolio assignment, broker credentials가 모두 원래 상태를 유지해야 한다. +- 실제 PostgreSQL에서 순차 owner transfer와 assignment/owner-transfer race를 검증한다. + +## 5. 비차단 정리 사항 + +1. 완전히 빈 PostgreSQL DB에 `ensure_schema()`만 실행하면 integration test가 `assets.tag_ids` 누락으로 실패한다. `pfhub migrate-db`로 migration head를 적용한 disposable DB에서는 통과했다. 테스트 실행 전제 또는 fixture provisioning을 명확히 해야 한다. +2. `implementation-progress.md`는 실제 구현 커밋이 존재함에도 상태를 “미커밋”으로 기록한다. 수정·재검증 후 현재 상태와 실제 명령 결과로 갱신해야 한다. +3. PATCH missing daemon, admin target-owner PATCH, deactivation 후 reference 보존과 token revoke 결합 시나리오의 직접 테스트를 보강하는 것이 좋다. +4. Starlette/httpx와 HTTP 422 상수 deprecation warning은 이번 blocker가 아니며 비차단 후속 항목이다. + +## 6. 리뷰 경계 + +- 제품 코드 수정: 수행하지 않음 +- worktree 구현 변경: 수행하지 않음 +- 커밋·push·PR·main 반영: 수행하지 않음 +- 배포·release: 수행하지 않음 +- 결과: **수정 후 동일 worktree에서 2차 QC 필요** diff --git a/.proofline/lines/line-0004/evidence/implementation-fix-rerequest.md b/.proofline/lines/line-0004/evidence/implementation-fix-rerequest.md new file mode 100644 index 0000000..2218e77 --- /dev/null +++ b/.proofline/lines/line-0004/evidence/implementation-fix-rerequest.md @@ -0,0 +1,226 @@ +# line-0004 구현 에이전트 수정 재요청 + +- **대상 이슈:** `fourblocks2020/portfolio-hub#27` +- **대상 Line:** `line-0004` +- **작업 worktree:** `/home/genichin/workspace/trex-lead/repos/portfolio-hub/.worktrees/line-0004` +- **작업 브랜치:** `line/line-0004-implementation` +- **수정 기준 커밋:** `e2102b6f2b855b7679ab67818202ef706d9848fb` +- **범위 권위:** `.proofline/lines/line-0004/dcy-0004.md` +- **1차 QC 결과:** `.proofline/lines/line-0004/evidence/first-qc-review.md` +- **요청 상태:** QC 반려 수정 요청 + +## 1. 작업 원칙 + +기존 구현의 의미와 승인된 Discovery 범위를 변경하지 말고 아래 blocker 및 관련 회귀 검증만 수정한다. 새로운 제품 결정, 다른 서비스 계약 영향, 승인 범위와의 충돌이 발견되면 구현을 중단하고 팀장에게 반환한다. + +`trex-web`은 명시적인 Out of Scope다. 해당 저장소의 코드·타입·화면·테스트를 변경하거나 실행하지 않는다. + +## 2. 필수 수정 — 참조 중 daemon owner transfer 차단 + +현재 daemon이 portfolio에 배정된 뒤 `update_daemon_owner()`로 daemon 소유자를 변경하면 다음 불변식이 깨진다. + +```text +daemon.owner_user_id == portfolio.owner_user_id +``` + +`update_daemon_owner()`에서 해당 daemon을 참조하는 portfolio가 하나라도 있으면 owner transfer가 원자적으로 실패하도록 수정한다. + +필수 결과: + +- 참조 중 daemon owner transfer: `409 Conflict` +- daemon owner: 변경되지 않음 +- portfolio `managed_by_daemon_id`: 변경되지 않음 +- 기존 broker credentials: 삭제되지 않음 +- transaction 일부만 반영되는 상태: 금지 + +다음 자동 동작은 승인 범위 밖이므로 구현하지 않는다. + +- portfolio assignment 자동 해제 +- portfolio를 새 daemon owner에게 자동 이전 +- 다른 daemon으로 자동 재배정 +- 기존 데이터 backfill + +## 3. 필수 수정 — assignment와 owner transfer 직렬화 + +portfolio assignment와 daemon owner transfer가 동일 daemon을 대상으로 동시에 실행될 때 일관된 row-lock 순서와 transaction 경계를 사용한다. + +허용되는 결과: + +1. owner transfer가 먼저 commit되면 이전 owner portfolio의 assignment는 foreign daemon으로 판단되어 실패한다. +2. assignment가 먼저 commit되면 owner transfer는 참조 충돌로 실패한다. + +금지되는 최종 상태: + +```text +portfolio.managed_by_daemon_id = daemon.id +portfolio.owner_user_id != daemon.owner_user_id +``` + +동시 실행이 timeout 또는 deadlock으로 끝나지 않도록 lock order를 검토한다. + +## 4. 도메인 및 API 오류 계약 + +참조 중 owner transfer 충돌을 daemon 미존재 오류와 구분한다. + +권장 구현 경계: + +- 전용 domain error 추가 +- repository `update_daemon_owner()`에서 전용 error 발생 +- `PATCH /v1/admin/daemons/{daemon_id}`에서 HTTP 409로 매핑 +- 기존 daemon 미존재 404 유지 +- 기존 admin 인증·권한 계약 유지 +- 미참조 owner transfer의 기존 credential invalidation 정책 유지 + +## 5. 필수 테스트 + +### 5.1 단위/API + +다음을 추가하거나 보강한다. + +- [ ] 미참조 daemon owner transfer 성공 +- [ ] 참조 중 daemon owner transfer `409` +- [ ] `409` 후 daemon owner 무변경 +- [ ] `409` 후 portfolio assignment 유지 +- [ ] `409` 후 broker credentials 유지 +- [ ] daemon 미존재 `404` 회귀 없음 +- [ ] PATCH missing daemon `404` 및 portfolio 무변경 +- [ ] admin PATCH에서 호출자 대신 target portfolio owner 기준 검증 +- [ ] deactivation 후 기존 assignment reference 보존 +- [ ] deactivation 시 기존 service token revoke 유지 + +### 5.2 실제 PostgreSQL + +migration head가 적용된 disposable PostgreSQL에서 검증한다. + +#### 순차 owner transfer + +1. owner A daemon 생성 +2. owner A portfolio에 daemon 배정 +3. daemon을 owner B로 이전 시도 +4. owner transfer 실패 +5. 동일 소유자 불변식 유지 +6. owner·assignment·credential 무변경 확인 + +#### assignment/owner-transfer race + +1. 별도 connection/thread에서 assignment와 owner transfer 동시 실행 +2. 두 실행 순서를 모두 유도하거나 반복 검증 +3. 허용된 결과만 발생하는지 확인 +4. 최종 DB 동일 소유자 불변식 확인 +5. thread timeout/deadlock 없음 확인 + +#### 미참조 owner transfer + +1. portfolio가 참조하지 않는 daemon owner transfer 성공 +2. 기존 broker credential invalidation 정책 유지 + +## 6. PostgreSQL 실행 전제 + +완전히 빈 DB에는 먼저 migration head를 적용한다. + +```bash +export TIMESCALEDB_HOST=127.0.0.1 +export TIMESCALEDB_PORT=5432 +export TIMESCALEDB_NAME= +export TIMESCALEDB_USER=portfolio_hub_user +export TIMESCALEDB_PASSWORD= +export TIMESCALEDB_SSLMODE=prefer +export PFHUB_DB_ENABLED=true + +.venv/bin/pfhub migrate-db + +PORTFOLIOHUB_RUN_POSTGRES_INTEGRATION=1 \ + .venv/bin/python -m pytest -q \ + tests/test_v1_portfolios_daemon_postgres_integration.py +``` + +실행 후 disposable DB를 삭제한다. 기존 공유·운영 DB를 오염시키지 않는다. + +## 7. 문서 및 evidence + +실제 동작과 일치하도록 필요한 범위에서 갱신한다. + +- `docs/interface-contract.md` +- `docs/data-model.md` +- daemon admin owner-transfer runtime flow 문서 +- `.proofline/lines/line-0004/evidence/implementation-progress.md` + +문서에 다음을 명시한다. + +- 참조 중 daemon owner transfer는 409 +- owner transfer와 assignment의 원자적 직렬화 +- 자동 assignment 해제·portfolio owner 변경 없음 +- 충돌 시 owner·assignment·credential partial mutation 없음 + +`implementation-progress.md`의 “미커밋” 표기를 실제 상태로 수정하고 다음 실행 근거를 기록한다. + +- QC 반려 blocker와 해결 방식 +- 수정 파일 목록 +- 수정 커밋 +- 집중 테스트 결과 +- 전체 회귀 결과 +- 실제 PostgreSQL 순차·경합 검증 결과 +- Ruff, compileall, diff check, ProofLine validate 결과 +- push·PR·main 반영·배포·release 미수행 경계 + +QC 재검증 전에는 implementation complete 또는 lifecycle 완료 문서를 새로 만들지 않는다. + +## 8. 최소 재검증 + +```bash +.venv/bin/python -m pytest -q \ + tests/test_v1_portfolios_daemon_assignment.py \ + tests/test_v1_portfolios_crud.py \ + tests/test_bat022_req053_admin_daemon_crud.py +``` + +```bash +.venv/bin/python -m pytest -q +``` + +```bash +PORTFOLIOHUB_RUN_POSTGRES_INTEGRATION=1 \ + .venv/bin/python -m pytest -q \ + tests/test_v1_portfolios_daemon_postgres_integration.py +``` + +```bash +ruff check \ + app/api/v1_portfolios.py \ + app/api/v1_admin_daemons.py \ + app/domain/errors.py \ + app/repositories/portfolio_repository.py \ + app/repositories/postgres.py \ + tests/test_v1_portfolios_daemon_assignment.py \ + tests/test_v1_portfolios_daemon_postgres_integration.py \ + tests/test_bat022_req053_admin_daemon_crud.py +``` + +```bash +.venv/bin/python -m compileall -q app tests +git diff --check +proofline validate +``` + +## 9. 금지 사항 + +- `trex-web` 변경·테스트 금지 +- approved Discovery 범위 변경 금지 +- 명시적 `managed_by_daemon_id: null` 해제 구현 금지 +- 자동 daemon 해제·이전·재배정·backfill 금지 +- main 작업 금지 +- push·PR·배포·release 금지 + +## 10. 반환 조건 + +수정·테스트·로컬 커밋 후 다음을 팀장에게 반환한다. + +1. 새 커밋 SHA +2. 변경 파일 목록 +3. blocker 해결 방식과 transaction/lock order +4. 테스트별 실제 pass/fail/skip 수 +5. 실제 PostgreSQL 순차·경합 결과 +6. 남은 비차단 위험 +7. worktree clean 여부 + +수정 완료는 QC 재검증 입력일 뿐 QC PASS나 lifecycle 완료를 의미하지 않는다. diff --git a/.proofline/lines/line-0004/evidence/implementation-progress.md b/.proofline/lines/line-0004/evidence/implementation-progress.md index cdd429e..9d9ad51 100644 --- a/.proofline/lines/line-0004/evidence/implementation-progress.md +++ b/.proofline/lines/line-0004/evidence/implementation-progress.md @@ -3,8 +3,8 @@ - **대상:** 포트폴리오 추가 시 연결 데몬을 설정해도 지정되지 않음 (GitHub Issue #27) - **브랜치:** `line/line-0004-implementation` - **기준:** `main` (`bb3bb5d3f0ce57358f7385084fba1343f7ff06e1`) -- **상태:** 로컬 구현·검증 완료 (미커밋) -- **최종 업데이트:** 2026-08-15 15:20 KST +- **상태:** 2차 QC 반려 수정 구현·검증·로컬 커밋 완료 (QC 재검증 대기) +- **최종 업데이트:** 2026-08-15 13:18 KST ### 구현 계획 및 작업 목록 @@ -26,6 +26,10 @@ - `docs/interface-contract.md`, `docs/data-model.md`에 daemon 배정 검증 계약·오류 코드·원자적 판정 경계를 반영한다. - [x] 회귀·정적 검사 및 진행 상황 업데이트 - 기존 portfolio 생성·수정의 인증·소유권·strategy·asset·disabled 계약 회귀, 전체 관련 테스트, compile·`git diff --check`·Ruff 검사를 실행하고 진행 상황 파일을 갱신한다. +- [x] 참조 중 daemon owner transfer 차단 및 409 계약 + - `update_daemon_owner()`가 daemon row를 `FOR UPDATE`로 잠근 뒤 참조 portfolio를 검사하고, 참조가 있으면 credential 삭제·owner 변경 전에 `DaemonInUseError`를 발생시킨다. admin PATCH는 이를 HTTP 409으로 반환한다. +- [x] assignment/owner-transfer 직렬화 및 실제 PostgreSQL 검증 + - assignment의 `FOR SHARE`와 transfer의 `FOR UPDATE`를 동일 daemon row에 적용해 transfer-first 또는 assignment-first의 허용 결과만 남기고, 순차·경합·credential·token-revoke 회귀를 실제 PostgreSQL로 검증한다. ### 진행 기록 @@ -36,5 +40,9 @@ - 2026-08-15 15:15 KST — 전용 API 테스트 `tests/test_v1_portfolios_daemon_assignment.py`(13개)를 추가했다. POST 성공·null 유지·malformed 422·미존재/foreign 404·inactive 409·admin target-owner 허용/거부, PATCH 지정·재지정·malformed·foreign·inactive를 검증해 통과했다. - 2026-08-15 15:18 KST — 실제 PostgreSQL 통합 테스트 `tests/test_v1_portfolios_daemon_postgres_integration.py`(4개)를 추가했다. HTTP `POST → GET` persistence, 검증 실패 시 portfolio 무변경, assignment vs deactivation race의 허용 outcome, PATCH 재배정 persistence를 실제 DB에서 검증했다. - 2026-08-15 15:20 KST — 회귀 검증: 전체 단위 스위트 `625 passed, 11 skipped`, 실제 PostgreSQL 통합(daemon 4 + strategy 4) `8 passed`. Python compile, `git diff --check`, 변경 핵심 파일 Ruff 검사 통과(Ruff 이슈 44건은 기준 커밋과 동일한 기존 항목이며 신규 도입 없음). +- 2026-08-15 13:10 KST — 1차 QC가 `update_daemon_owner()`가 참조 중 daemon의 소유자를 변경해 `daemon.owner_user_id == portfolio.owner_user_id` 불변식을 깨뜨리는 blocker를 재현해 FAIL 판정했다. `.proofline/lines/line-0004/evidence/implementation-fix-rerequest.md`의 명시 요구에 따라 2차 보완 계획을 적용했다. +- 2026-08-15 13:18 KST — `DaemonInUseError`를 추가하고 `update_daemon_owner()`를 daemon `FOR UPDATE` → 참조 portfolio 검사 → credential 삭제 → owner UPDATE 순서의 단일 트랜잭션으로 변경했다. assignment는 기존 daemon `FOR SHARE`를 먼저 획득하므로 transfer-first면 이전 owner assignment가 foreign으로 실패하고, assignment-first면 transfer가 409으로 실패한다. 자동 assignment 해제·portfolio owner 이전·재배정·backfill은 추가하지 않았다. +- 2026-08-15 13:18 KST — admin daemon PATCH 409 매핑, PATCH missing daemon 404·무변경, 참조 transfer의 owner/assignment/credential 무변경, 미참조 transfer의 credential invalidation, assignment/transfer race, deactivation 후 assignment reference 보존과 service-token revoke를 테스트로 보강했다. +- 2026-08-15 13:18 KST — 수정 커밋 `e90519a fix(daemons): block owner transfer while assigned`를 생성했다. 집중 단위/API `62 passed`, 전체 스위트 `627 passed, 15 skipped`, 실제 PostgreSQL daemon integration `8 passed`, `compileall`, `git diff --check`, `proofline validate`를 통과했다. 지정 Ruff 검사는 기준 `e2102b6`의 기존 `76`건에서 현재 `74`건으로 감소했으며 새 lint 항목은 없다. -> 각 작업이 실제로 완료되고 검증될 때마다 이 댓글의 체크박스와 진행 기록을 갱신한다. 체크 표시는 구현 완료 또는 해당 검증의 실제 통과를 의미하며, 배포·릴리스 완료를 의미하지 않는다. 현재 변경은 기준 HEAD 위 미커밋 상태로 보존되어 있으며 커밋·push·PR·main 반영·배포는 수행하지 않았다. \ No newline at end of file +> 각 작업이 실제로 완료되고 검증될 때마다 이 댓글의 체크박스와 진행 기록을 갱신한다. 체크 표시는 구현 완료 또는 해당 검증의 실제 통과를 의미하며, 배포·릴리스 완료를 의미하지 않는다. 1차 구현은 `e2102b6`, 2차 QC 반려 수정은 `e90519a`에 로컬 커밋되었다. push·PR·main 반영·배포·release는 수행하지 않았으며, 이 기록은 QC PASS 또는 lifecycle 완료를 뜻하지 않는다.