개발 기록 2026-10-20 docs/devlog/2026-10-20-3-Review와-직무분리.md GitHub에서 보기

Review와 직무분리

목표

작성자의 결론 제출과 검토자의 승인/반려를 구현한다. 화면이 가진 오래된 상태나 결론을 그대로 믿지 않고, 역할과 결론 작성자를 확인해 직무분리를 지킨다. 상태 변경과 감사 이벤트는 함께 커밋하고 알림은 그 뒤에 보낸다.

완료 조건은 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개
이 문서 결정, 검증 결과와 제한 기록

결론 제출

  1. finding을 ID로 다시 읽고 run, 전표를 preload한다. DB에서 읽은 engagement로 authorize(:submit_conclusion)을 호출한다.
  2. Ecto.Multi의 첫 단계에서 finding을 FOR UPDATE로 잠근다. open, concluded, rejected에서 제출할 수 있고 approved는 :invalid_state다.
  3. finding_id와 preparer_id는 컨텍스트에서 넣는다. 폼의 같은 이름 필드는 받지 않는다. ai_triage_id가 있으면 UUID 형식과 같은 finding 소속인지 확인한다.
  4. conclusion을 새 행으로 넣고 finding을 concluded로 바꾸며 conclusion.submitted를 함께 쓴다. payload에는 결론 ID, 판단, AI 제안 ID 또는 null, DB에서 읽은 전표번호가 들어간다.
  5. Repo.transact가 성공한 뒤 finding_updated, AuditTrail.broadcast_appended/1을 차례로 보낸다. 결론 시각은 스키마의 inserted_at을 사용한다.

승인과 반려

  1. 비멤버는 이벤트 없이 :forbidden이다. 작성자 역할의 검토 시도는 review.denied를 record/1로 기록하고 감사 알림을 보낸 뒤 :forbidden이다.
  2. 검토자는 트랜잭션 안에서 finding을 먼저 잠근다. 현재 상태가 concluded가 아니면 :invalid_state다.
  3. inserted_at DESC, id DESC로 최신 결론을 고른다. 화면이 넘긴 결론 ID와 다르거나 ID 형식이 틀리면 :stale이다. 자기 결론이면 트랜잭션을 롤백하고 바깥에서 review.denied를 남긴 뒤 :self_review다.
  4. preparer_id, finding_id, conclusion_id는 조회한 결론에서, reviewer_id는 scope에서 가져온다. ReviewDecision 삽입, finding 상태 변경, review.approved 또는 review.rejected를 같은 Multi에서 실행한다.
  5. 반려 코멘트는 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_id changeset 오류로 반환하도록 고쳤다. 재실행은 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 기반 통합 테스트에서 다룬다.
  • 계약 변경 요청: 없음. 공유 파일과 메인 소유 스키마 변경 요청도 없음.

관련 커밋

  • 6757f79 feat(review): 결론 제출과 직무분리 검토 구현
  • 이 문서: docs(devlog): Review와 직무분리 검증 기록