Review와 직무분리
- 작성일: 2026-10-20
- 작성: 워커 W2 (Codex
gpt-6-astra, effort xhigh) - 브랜치:
feature/m4-review - WBS: 4.1 Review 컨텍스트와 직무분리
- 관련 문서: 시스템 구조와 데이터 모델 (2.1, 3.9-3.12, 4, 6, 8, 12.2), 분개 테스트 룰과 감사 용어 (5장), 라운드 4 계약과 통합, Testing 실행 경로
목표
작성자의 결론 제출과 검토자의 승인/반려를 구현한다. 화면이 가진 오래된 상태나 결론을 그대로 믿지 않고, 역할과 결론 작성자를 확인해 직무분리를 지킨다. 상태 변경과 감사 이벤트는 함께 커밋하고 알림은 그 뒤에 보낸다.
완료 조건은 submit_conclusion/3, decide/4, 컨텍스트와 DB 제약을 구분한 테스트, MIX_TEST_PARTITION=3 mix precommit 통과, devlog와 구현 커밋이다.
한 일
| 파일 | 변경 |
|---|---|
lib/tickmark/review.ex |
결론 제출, 승인/반려, 최신 결론 확인, 직무분리, 감사 이벤트와 알림 |
test/tickmark/review_test.exs |
31개 테스트. 제출 11개, 검토 14개, DB 제약 5개, 없는 finding 1개 |
| 이 문서 | 결정, 검증 결과와 제한 기록 |
결론 제출
- finding을 ID로 다시 읽고 run, 전표를 preload한다. DB에서 읽은 engagement로
authorize(:submit_conclusion)을 호출한다. Ecto.Multi의 첫 단계에서 finding을FOR UPDATE로 잠근다.open,concluded,rejected에서 제출할 수 있고approved는:invalid_state다.finding_id와preparer_id는 컨텍스트에서 넣는다. 폼의 같은 이름 필드는 받지 않는다.ai_triage_id가 있으면 UUID 형식과 같은 finding 소속인지 확인한다.- conclusion을 새 행으로 넣고 finding을
concluded로 바꾸며conclusion.submitted를 함께 쓴다. payload에는 결론 ID, 판단, AI 제안 ID 또는 null, DB에서 읽은 전표번호가 들어간다. Repo.transact가 성공한 뒤finding_updated,AuditTrail.broadcast_appended/1을 차례로 보낸다. 결론 시각은 스키마의inserted_at을 사용한다.
승인과 반려
- 비멤버는 이벤트 없이
:forbidden이다. 작성자 역할의 검토 시도는review.denied를record/1로 기록하고 감사 알림을 보낸 뒤:forbidden이다. - 검토자는 트랜잭션 안에서 finding을 먼저 잠근다. 현재 상태가
concluded가 아니면:invalid_state다. inserted_at DESC, id DESC로 최신 결론을 고른다. 화면이 넘긴 결론 ID와 다르거나 ID 형식이 틀리면:stale이다. 자기 결론이면 트랜잭션을 롤백하고 바깥에서review.denied를 남긴 뒤:self_review다.preparer_id,finding_id,conclusion_id는 조회한 결론에서,reviewer_id는 scope에서 가져온다. ReviewDecision 삽입, finding 상태 변경,review.approved또는review.rejected를 같은 Multi에서 실행한다.- 반려 코멘트는 changeset에서 필수 검증하고 반려 이벤트 payload에도 넣는다. 성공 알림은 커밋 뒤에만 보낸다. 결정 시각은 스키마의
inserted_at이다.
검증 범위
- 작성자 성공, 검토자/비멤버 제출 거부,
open/concluded/rejected제출,approved종결, 재제출 시 기존 결론 보존. - 화면의 오래된 status/triage_status와 조작된 engagement/작성자 필드를 신뢰하지 않는지 확인.
- AI 제안이 없어도 제출 가능하고 triage 기준 미만 finding도 제출 가능. 잘못된 UUID, 없는 제안, 다른 finding의 제안은 changeset 오류.
- 승인/반려의 행과 상태, 사용자 actor, payload 전체, 두 알림의 필드 및 중복 부재.
- 코멘트나 결정 오류 시 행, 상태, 이벤트, 알림에 변화 없음.
- 오래된 결론, 같은 시각 결론의 ID 정렬, 잘못된 결론 ID,
invalid_state -> stale -> self_review검사 우선순위. - 작성자 역할 거부와 자기 결론 검토 거부의 감사 기록. 잘못된 attempted 값은 null. 비멤버는 감사 기록 없음.
- 같은 결론에 두 번째 결정은 오래된 finding struct를 넘겨도
:invalid_state. - DB 직접 삽입으로
reviewer_not_preparer,review_decisions_conclusion_id_fkey,comment_required_on_reject,decision_valid,review_decisions_conclusion_id_index를 각각 위반시키고 오류의constraint_name을 단언.
결정과 트레이드오프
- finding을 먼저 잠근다. 자식인 conclusions/review_decisions를 먼저 넣으면 FK 검사에서 부모의 key-share lock을 잡는다. 다른 요청과 락 획득 순서가 엇갈리지 않도록 계약과 runner의 lock-first 순서를 따랐다. 화면의 status로 미리 결정하는 대안은 다른 요청이 이미 승인한 finding을 다시 제출하거나 결정할 수 있어 쓰지 않았다.
- 오류 판정 순서를 유지한다. 결론 ID가 틀려도 비멤버/역할, 현재 상태를 먼저 검사한다. 검토자가 보지 않은 최신 결론을 자동 승인하는 대신
:stale을 돌려 화면이 다시 읽게 한다. 동일한 시각은 ID 내림차순으로 정렬해 결과를 고정했다. - 자기 검토 거부 이벤트는 롤백 뒤에 쓴다. 상태 변경 트랜잭션에 거부 이벤트를 넣으면 롤백과 함께 사라진다.
record/1로 따로 저장한 이벤트만 감사 알림으로 보낸다. finding 상태는 바뀌지 않아finding_updated는 보내지 않는다. - 기존 스키마의 constraint 매핑을 재사용한다. ReviewDecision에 CHECK, 복합 FK, unique 매핑이 이미 있어 컨텍스트에 중복 매핑을 추가하지 않았다. 앱 스키마와 마이그레이션은 수정하지 않았다.
- DB 테스트는 유효성 검증을 일부러 우회한다.
change/2에 constraint 매핑만 붙여 컨텍스트 없이 삽입한다.decision_valid는 Ecto.Enum의 dump 검증도 우회해야 하므로 같은 테스트 파일에 decision을 문자열로 쓰는 작은 테스트 전용 스키마를 두었다. Ecto 단계에서 거절됐다는 사실을 DB 제약의 증거로 대신하지 않는다.
막힌 점과 해결
- 첫 Review 테스트 실행은
Result: 30/31 passed였다. 잘못된ai_triage_id문자열을 changeset이 받았고, 이어지는Repo.exists?가Ecto.Query.CastError를 냈다. 조회 전에Ecto.UUID.cast/1로 형식을 확인해:ai_triage_idchangeset 오류로 반환하도록 고쳤다. 재실행은Result: 31 passed였다. - DB 제약 테스트는
Repo.insert(..., mode: :savepoint)를 쓴다. 실패한 SQL의 영향만 savepoint까지 되돌려 같은 테스트 트랜잭션에서 null, 빈 문자열, 공백 코멘트를 각각 검사할 수 있다.
동시성 검증의 제한
두 검토자의 실제 별도 연결 경합 테스트는 추가하지 않았다. 기존 runner 동시성 테스트는 감사 이벤트를 만들지 않는 경로만 커밋하고, 끝나면 fixture를 삭제한다. Review 성공에는 불변 conclusions와 review_decisions 및 append-only audit_events가 필요해 같은 정리 방식을 쓸 수 없다. 실제 트리거가 conclusions/review_decisions의 UPDATE/DELETE와 audit_events의 UPDATE/DELETE/TRUNCATE를 막는 것을 확인했다.
Sandbox에서 Task가 연결을 공유하면 실제 DB 락 경합과 커밋 가시성의 증거가 되지 않는다. 이번 검증은 순차 중복 결정, 오래된 상태/결론, unique 제약과 lock-first 구현 확인까지다. 트리거를 끄거나 partition DB에 영구 행을 남기는 대신, 실제 경합 검증은 폐기 가능한 별도 DB를 쓰는 통합 테스트로 남겼다. 두 알림은 컨텍스트 성공 반환 뒤 수신을 확인했으며, 별도 연결에서 커밋 가시성을 검증한 것은 아니다.
검증
실제 출력:
$ MIX_TEST_PARTITION=3 mix test test/tickmark/review_test.exs
Finished in 0.6 seconds (0.6s async, 0.00s sync)
Result: 31 passed
$ MIX_TEST_PARTITION=3 mix precommit
Finished in 9.8 seconds (9.1s async, 0.7s sync)
Result: 420 passed
precommit성공, 컴파일 경고 0. 기존 389개와 Review 31개를 포함한다.- 문서 작성 뒤 최종
precommit재실행도Finished in 6.1 seconds (5.3s async, 0.7s sync),Result: 420 passed였다. mix.lock무변경,git diff --check와 스테이징 후git diff --cached --check통과.- Application env를 바꾸지 않았고 테스트 DB partition은 3만 사용했다.
- 운영 반영은 이 작업 범위에 없으며 수행하지 않았다.
Elixir 학습 포인트
- Multi의 이름 붙인 단계:
:locked_finding,:conclusion,:review_decision,:finding,:audit의 성공 값이 다음 함수에 맵으로 전달된다. 뒤 단계가 실패하면 앞 단계의 DB 쓰기도 되돌아간다. 함수로 changeset을 만들면 먼저 잠근 finding을 기준으로 검증할 수 있다. - 패턴 매칭과 오류 구분:
{:error, :conclusion, :self_review, _}만 따로 처리해 롤백 뒤 거부 로그를 남긴다. 나머지는{:error, reason}으로 반환한다. 상태/권한 오류와 폼 검증 오류를 호출자가 구분할 수 있다. - 변하지 않는 결론과 최신성: 기존 conclusion을 update하지 않고 새 행을 쌓는다.
inserted_at의 마이크로초 정밀도와 ID 정렬로 최신 행을 정하고, 화면이 본 ID를 비교해 낡은 화면의 결정을 막는다. - changeset과 DB 제약은 서로 다른 방어선: changeset은 정상 요청에 이해 가능한 오류를 주고, DB CHECK/FK/unique는 검증을 우회한 삽입도 거절한다.
check_constraint는 CHECK를 실행하는 함수가 아니라 DB가 돌려준 제약 오류를 changeset에 연결하는 선언이다. - binary_id 형식 확인: changeset의 기본 binary_id 캐스팅만으로 UUID 문자열이 검증됐다고 가정하지 않는다. 사용자 입력을 UUID 쿼리에 쓰기 전에
Ecto.UUID.cast/1로 확인한다.
다음 할 일
- 메인 세션이 이 브랜치를 통합하고
docs/README.md에 문서를 등록한다. - 라운드 4b 화면은 현재 표시한 conclusion ID를
decide/4에 전달하고,:stale이면 최신 결론을 다시 읽게 한다. - 실제 두 연결의 검토 경합 검증은 폐기 가능한 별도 DB 기반 통합 테스트에서 다룬다.
- 계약 변경 요청: 없음. 공유 파일과 메인 소유 스키마 변경 요청도 없음.
관련 커밋
6757f79feat(review): 결론 제출과 직무분리 검토 구현- 이 문서: docs(devlog): Review와 직무분리 검증 기록