103 lines
5.5 KiB
Markdown
103 lines
5.5 KiB
Markdown
# 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 댓글:** <https://github.com/fourblocks2020/portfolio-hub/issues/27#issuecomment-5300441199>
|
|
|
|
## 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=<same daemon id>
|
|
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 필요**
|