diff --git a/backend/alembic/versions/0015_review_models.py b/backend/alembic/versions/0015_review_models.py new file mode 100644 index 00000000..dc31489f --- /dev/null +++ b/backend/alembic/versions/0015_review_models.py @@ -0,0 +1,50 @@ +"""review_models: Review and ReviewFinding tables + +Revision ID: 0015_review_models +Revises: 0014_post_submit_setup +Create Date: 2026-07-11 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + +revision = "0015_review_models" +down_revision = "0014_post_submit_setup" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "reviews", + sa.Column("id", sa.String(36), primary_key=True), + sa.Column("submission_id", sa.String(36), sa.ForeignKey("submissions.id"), nullable=False, index=True), + sa.Column("reviewer_actor_id", sa.String(100), sa.ForeignKey("actor_identities.actor_id"), nullable=False, index=True), + sa.Column("decision", sa.String(30), nullable=False, index=True), + sa.Column("acceptance_evidence_refs", sa.JSON, nullable=False, server_default="[]"), + sa.Column("comment", sa.Text, nullable=True), + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now()), + sa.Column("updated_at", sa.DateTime(timezone=True), server_default=sa.func.now()), + sa.CheckConstraint("decision in ('accept', 'needs_revision', 'reject')", name="ck_reviews_decision"), + ) + op.create_index("ix_reviews_submission_created", "reviews", ["submission_id", "created_at"]) + + op.create_table( + "review_findings", + sa.Column("id", sa.String(36), primary_key=True), + sa.Column("review_id", sa.String(36), sa.ForeignKey("reviews.id"), nullable=False, index=True), + sa.Column("severity", sa.String(20), nullable=False), + sa.Column("area", sa.String(100), nullable=False), + sa.Column("issue", sa.Text, nullable=False), + sa.Column("required_fix", sa.Text, nullable=False), + sa.Column("evidence_ref", sa.String(500), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now()), + sa.CheckConstraint("severity in ('low', 'medium', 'high', 'critical')", name="ck_review_findings_severity"), + ) + + +def downgrade() -> None: + op.drop_table("review_findings") + op.drop_table("reviews") diff --git a/backend/app/api/router.py b/backend/app/api/router.py index 0003af9c..c2b57abc 100644 --- a/backend/app/api/router.py +++ b/backend/app/api/router.py @@ -8,6 +8,7 @@ from app.api.routes.health import router as health_router from app.modules.checkers.router import router as checkers_router from app.modules.projects.router import router as projects_router +from app.modules.review.router import router as reviews_router from app.modules.tasks.router import router as tasks_router api_router = APIRouter() @@ -15,5 +16,6 @@ api_router.include_router(health_router, prefix="/api/v1") api_router.include_router(auth_router, prefix="/api/v1") api_router.include_router(projects_router, prefix="/api/v1") +api_router.include_router(reviews_router, prefix="/api/v1") api_router.include_router(tasks_router, prefix="/api/v1") api_router.include_router(checkers_router, prefix="/api/v1") diff --git a/backend/app/db/models.py b/backend/app/db/models.py index 1f325f76..6e9a69fd 100644 --- a/backend/app/db/models.py +++ b/backend/app/db/models.py @@ -17,6 +17,7 @@ ReviewPolicy, SubmissionArtifactPolicy, ) +from app.modules.review.models import Review, ReviewFinding # noqa: F401 from app.modules.tasks.models import ( # noqa: F401 AuditEvent, EvidenceItem, diff --git a/backend/app/modules/review/models.py b/backend/app/modules/review/models.py new file mode 100644 index 00000000..dd32833a --- /dev/null +++ b/backend/app/modules/review/models.py @@ -0,0 +1,87 @@ +"""SQLAlchemy models for review decisions and findings.""" + +from __future__ import annotations + +from datetime import datetime + +from sqlalchemy import CheckConstraint, DateTime, ForeignKey, Index, JSON, String, Text +from sqlalchemy.orm import Mapped, mapped_column, relationship +from sqlalchemy.sql import func + +from app.db.base import Base + +REVIEW_DECISION_ACCEPT = "accept" +REVIEW_DECISION_NEEDS_REVISION = "needs_revision" +REVIEW_DECISION_REJECT = "reject" +REVIEW_DECISIONS = ( + REVIEW_DECISION_ACCEPT, + REVIEW_DECISION_NEEDS_REVISION, + REVIEW_DECISION_REJECT, +) +REVIEW_FINDING_SEVERITIES = ("low", "medium", "high", "critical") + + +class Review(Base): + """Auditable human review decision for one immutable submission.""" + + __tablename__ = "reviews" + __table_args__ = ( + CheckConstraint( + "decision in ('accept', 'needs_revision', 'reject')", + name="ck_reviews_decision", + ), + Index("ix_reviews_submission_created", "submission_id", "created_at"), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True) + submission_id: Mapped[str] = mapped_column( + ForeignKey("submissions.id"), + nullable=False, + index=True, + ) + reviewer_actor_id: Mapped[str] = mapped_column( + ForeignKey("actor_identities.actor_id"), + nullable=False, + index=True, + ) + decision: Mapped[str] = mapped_column(String(30), nullable=False, index=True) + acceptance_evidence_refs: Mapped[list[str]] = mapped_column( + "acceptance_evidence_refs", + nullable=False, + default=list, + ) + comment: Mapped[str | None] = mapped_column(Text) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + server_default=func.now(), + onupdate=func.now(), + ) + + findings: Mapped[list[ReviewFinding]] = relationship( + back_populates="review", + cascade="all, delete-orphan", + ) + + +class ReviewFinding(Base): + """Structured reviewer finding attached to a review decision.""" + + __tablename__ = "review_findings" + __table_args__ = ( + CheckConstraint( + "severity in ('low', 'medium', 'high', 'critical')", + name="ck_review_findings_severity", + ), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True) + review_id: Mapped[str] = mapped_column(ForeignKey("reviews.id"), nullable=False, index=True) + severity: Mapped[str] = mapped_column(String(20), nullable=False) + area: Mapped[str] = mapped_column(String(100), nullable=False) + issue: Mapped[str] = mapped_column(Text, nullable=False) + required_fix: Mapped[str] = mapped_column(Text, nullable=False) + evidence_ref: Mapped[str] = mapped_column(String(500), nullable=False) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + + review: Mapped[Review] = relationship(back_populates="findings") diff --git a/backend/app/modules/review/router.py b/backend/app/modules/review/router.py new file mode 100644 index 00000000..376e3791 --- /dev/null +++ b/backend/app/modules/review/router.py @@ -0,0 +1,61 @@ +"""FastAPI routes for review decision creation and querying.""" + +from __future__ import annotations + +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException, status +from sqlalchemy.ext.asyncio import AsyncSession + +from app.api.deps.auth import get_registered_actor +from app.db.session import get_db_session +from app.modules.review.schemas import ReviewCreate, ReviewSchema +from app.modules.review.service import ReviewError, ReviewService, ReviewValidationError +from app.schemas.auth import ActorContext + +router = APIRouter(prefix="/reviews", tags=["reviews"]) + + +@router.post("", response_model=ReviewSchema, status_code=status.HTTP_201_CREATED) +async def create_review( + payload: ReviewCreate, + actor: Annotated[ActorContext, Depends(get_registered_actor)], + session: Annotated[AsyncSession, Depends(get_db_session)], +) -> ReviewSchema: + """Submit a human review decision with structured findings. + + Enforces: + - Only accept, needs_revision, reject. + - reject / needs_revision require at least one finding. + - accept requires evidence references and no blocking checker results. + """ + try: + return await ReviewService(session).create_review(payload, actor.actor_id) + except ReviewValidationError as exc: + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc)) + except ReviewError as exc: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) + + +@router.get("/{submission_id}", response_model=list[ReviewSchema]) +async def list_reviews_for_submission( + submission_id: str, + actor: Annotated[ActorContext, Depends(get_registered_actor)], + session: Annotated[AsyncSession, Depends(get_db_session)], +) -> list[ReviewSchema]: + """Return all review decisions for a submission, newest first.""" + reviews = await ReviewService(session).get_reviews_for_submission(submission_id) + return list(reviews) + + +@router.get("/review/{review_id}", response_model=ReviewSchema) +async def get_review( + review_id: str, + actor: Annotated[ActorContext, Depends(get_registered_actor)], + session: Annotated[AsyncSession, Depends(get_db_session)], +) -> ReviewSchema: + """Return one review by its id.""" + review = await ReviewService(session).get_review(review_id) + if not review: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Review not found") + return review diff --git a/backend/app/modules/review/schemas.py b/backend/app/modules/review/schemas.py new file mode 100644 index 00000000..6b70d525 --- /dev/null +++ b/backend/app/modules/review/schemas.py @@ -0,0 +1,57 @@ +"""Pydantic schemas for review decisions and findings.""" + +from __future__ import annotations + +from datetime import datetime + +from pydantic import BaseModel, Field, ConfigDict + +from app.modules.review.models import ( + REVIEW_DECISION_ACCEPT, + REVIEW_DECISION_NEEDS_REVISION, + REVIEW_DECISION_REJECT, + REVIEW_DECISIONS, + REVIEW_FINDING_SEVERITIES, +) + +class ReviewFindingSchema(BaseModel): + """Schema for a single review finding.""" + model_config = ConfigDict(from_attributes=True) + + id: str + severity: str = Field(..., description="Severity of the finding (low, medium, high, critical)") + area: str = Field(..., description="The functional area the finding relates to") + issue: str = Field(..., description="Detailed description of the issue") + required_fix: str = Field(..., description="What must be fixed to resolve this finding") + evidence_ref: str = Field(..., description="Reference to evidence or file location") + created_at: datetime + +class ReviewSchema(BaseModel): + """Schema for a complete review decision.""" + model_config = ConfigDict(from_attributes=True) + + id: str + submission_id: str + reviewer_actor_id: str + decision: str + acceptance_evidence_refs: list[str] = Field(default_factory=list) + comment: str | None = None + findings: list[ReviewFindingSchema] = [] + created_at: datetime + updated_at: datetime + +class ReviewFindingCreate(BaseModel): + """Input schema for creating a finding.""" + severity: str = Field(..., pattern="^(low|medium|high|critical)$") + area: str = Field(..., min_length=1, max_length=100) + issue: str = Field(..., min_length=1) + required_fix: str = Field(..., min_length=1) + evidence_ref: str = Field(..., min_length=1) + +class ReviewCreate(BaseModel): + """Input schema for creating a review.""" + submission_id: str = Field(..., min_length=1) + decision: str = Field(..., pattern="^(accept|needs_revision|reject)$") + acceptance_evidence_refs: list[str] = Field(default_factory=list) + comment: str | None = None + findings: list[ReviewFindingCreate] = Field(default_factory=list) diff --git a/backend/app/modules/review/service.py b/backend/app/modules/review/service.py new file mode 100644 index 00000000..c38dd404 --- /dev/null +++ b/backend/app/modules/review/service.py @@ -0,0 +1,159 @@ +"""Service for managing structured review decisions and findings.""" + +from __future__ import annotations + +import uuid +from typing import Sequence + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import selectinload + +from app.db.models import AuditEvent +from app.modules.checkers.models import CheckerResult +from app.modules.review.models import ( + REVIEW_DECISION_ACCEPT, + REVIEW_DECISION_NEEDS_REVISION, + REVIEW_DECISION_REJECT, + Review, + ReviewFinding, +) +from app.modules.review.schemas import ReviewCreate, ReviewSchema +from app.modules.tasks.models import Submission + + +class ReviewError(Exception): + """Base exception for review operations.""" + pass + + +class ReviewValidationError(ReviewError): + """Raised when a review decision fails validation rules.""" + pass + + +class ReviewService: + """Creates and queries structured review decisions with findings.""" + + def __init__(self, session: AsyncSession) -> None: + self.session = session + + async def create_review(self, review_in: ReviewCreate, actor_id: str) -> ReviewSchema: + """Create a review decision with full validation. + + Enforcement: + - needs_revision / reject require at least one finding. + - accept requires acceptance_evidence_refs and no blocking checker results. + """ + # 1. Verify submission exists + submission = await self.session.get(Submission, review_in.submission_id) + if not submission: + raise ReviewError(f"Submission {review_in.submission_id} not found") + + # 2. Validate decision rules + self._validate_decision_requirements(review_in) + + # 3. Check for blocking checker results (accept only) + if review_in.decision == REVIEW_DECISION_ACCEPT: + await self._verify_no_blocking_results(submission.id) + + # 4. Create Review + review_id = str(uuid.uuid4()) + review = Review( + id=review_id, + submission_id=review_in.submission_id, + reviewer_actor_id=actor_id, + decision=review_in.decision, + acceptance_evidence_refs=review_in.acceptance_evidence_refs, + comment=review_in.comment, + ) + self.session.add(review) + + # 5. Create findings + for f in review_in.findings: + finding = ReviewFinding( + id=str(uuid.uuid4()), + review_id=review_id, + severity=f.severity, + area=f.area, + issue=f.issue, + required_fix=f.required_fix, + evidence_ref=f.evidence_ref, + ) + self.session.add(finding) + + # 6. Audit event + audit = AuditEvent( + id=str(uuid.uuid4()), + entity_type="review", + entity_id=review_id, + event_type="review_decision_created", + actor_id=actor_id, + external_subject=review_in.submission_id, + external_issuer="workstream-backend", + actor_roles=["reviewer"], + claim_snapshot={}, + auth_source="internal", + is_dev_auth=False, + reason=f"Review decision {review_in.decision} created", + event_payload={ + "decision": review_in.decision, + "finding_count": len(review_in.findings), + }, + ) + self.session.add(audit) + + await self.session.flush() + # Eager-load findings before returning + result = await self.session.get(Review, review_id, options=[selectinload(Review.findings)]) + return ReviewSchema.model_validate(result) + + def _validate_decision_requirements(self, review_in: ReviewCreate) -> None: + """Enforce findings for reject / needs_revision and evidence for accept.""" + if review_in.decision in (REVIEW_DECISION_REJECT, REVIEW_DECISION_NEEDS_REVISION): + if not review_in.findings: + raise ReviewValidationError( + f"Decision '{review_in.decision}' requires at least one structured finding.", + ) + elif review_in.decision == REVIEW_DECISION_ACCEPT: + if not review_in.acceptance_evidence_refs: + raise ReviewValidationError( + "Decision 'accept' requires at least one acceptance evidence reference.", + ) + + async def _verify_no_blocking_results(self, submission_id: str) -> None: + """Ensure no CheckerResult has blocks_review=True for this submission.""" + stmt = select(CheckerResult).where( + CheckerResult.submission_id == submission_id, + CheckerResult.blocks_review == True, + ) + res = await self.session.execute(stmt) + blockers = res.scalars().all() + if blockers: + raise ReviewValidationError( + f"Submission cannot be accepted while {len(blockers)} blocking checker result(s) exist.", + ) + + async def get_review(self, review_id: str) -> ReviewSchema | None: + stmt = ( + select(Review) + .options(selectinload(Review.findings)) + .where(Review.id == review_id) + ) + res = await self.session.execute(stmt) + review = res.scalar_one_or_none() + if not review: + return None + return ReviewSchema.model_validate(review) + + async def get_reviews_for_submission( + self, submission_id: str, + ) -> Sequence[ReviewSchema]: + stmt = ( + select(Review) + .options(selectinload(Review.findings)) + .where(Review.submission_id == submission_id) + .order_by(Review.created_at.desc()) + ) + res = await self.session.execute(stmt) + return [ReviewSchema.model_validate(r) for r in res.scalars().all()] diff --git a/backend/tests/test_review.py b/backend/tests/test_review.py new file mode 100644 index 00000000..8a1fd27f --- /dev/null +++ b/backend/tests/test_review.py @@ -0,0 +1,351 @@ +"""Tests for structured review decisions and findings (Issue #34).""" + +from __future__ import annotations + +import asyncio +from collections.abc import AsyncIterator, Iterator +from uuid import uuid4 + +import pytest +from httpx import ASGITransport, AsyncClient +from sqlalchemy import select + +from app.core.config import get_settings +from app.db import session as db_session +from app.main import create_app +from app.modules.checkers.models import CheckerResult, CheckerRun +from app.modules.review.models import ( + REVIEW_DECISION_ACCEPT, + REVIEW_DECISION_NEEDS_REVISION, + REVIEW_DECISION_REJECT, +) +from app.modules.tasks.models import AuditEvent, Submission, WorkstreamTask + +from tests.test_tasks import ( + auth_headers, + create_active_project, + create_started_task, + set_dev_actor, +) + + +# ── database fixtures (same pattern as test_tasks / test_checkers) ── + + +@pytest.fixture +def review_database_env( + monkeypatch: pytest.MonkeyPatch, + postgres_database_url: str, + migration_lock, +) -> Iterator[str]: + """Fresh database migrated to head, downgraded after each test class/module.""" + import asyncio + from pathlib import Path + from alembic import command + from alembic.config import Config + + monkeypatch.setenv("WORKSTREAM_DATABASE_URL", postgres_database_url) + monkeypatch.setenv("WORKSTREAM_CELERY_TASK_ALWAYS_EAGER", "true") + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="review-test-subject") + get_settings.cache_clear() + + from app.db import session as db_session + asyncio.run(db_session.dispose_engine()) + + project_root = Path(__file__).resolve().parents[1] + config = Config(str(project_root / "alembic.ini")) + config.set_main_option("script_location", str(project_root / "alembic")) + + with migration_lock(): + command.downgrade(config, "base") + command.upgrade(config, "head") + yield postgres_database_url + command.downgrade(config, "base") + asyncio.run(db_session.dispose_engine()) + get_settings.cache_clear() + + +@pytest.fixture +async def review_client(review_database_env: str) -> AsyncIterator[AsyncClient]: + """httpx async client wired to the FastAPI app.""" + app = create_app() + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + yield client + + +# ── helpers ── + + +async def _create_locked_submission(client: AsyncClient, monkeypatch: pytest.MonkeyPatch) -> dict: + """Return a locked submission that is ready for review.""" + project = await create_active_project(client) + task = await create_started_task(client, project["id"], monkeypatch) + submission_resp = await client.post( + f"/api/v1/tasks/{task['id']}/submit", + headers=auth_headers(), + json={ + "summary": "test submission", + "package_hash": "sha256:test", + "worker_attestation": "I attest this is original.", + "artifact_hash_manifest": [], + "evidence_items": [{"type": "log", "label": "test", "uri": "local://test", "hash": "sha256:test"}], + }, + ) + assert submission_resp.status_code == 201, submission_resp.text + submission = submission_resp.json() + # finalize submission to lock it + finalize_resp = await client.post( + f"/api/v1/tasks/{task['id']}/submissions/{submission['id']}/finalize", + headers=auth_headers(), + json={"reason": "ready for review"}, + ) + # finalize may succeed or already be locked + assert finalize_resp.status_code in (200, 409), finalize_resp.text + return submission + + +def _finding_payload(severity: str = "medium") -> dict: + """Build a minimal finding payload for review creation.""" + return { + "severity": severity, + "area": "logic", + "issue": "incorrect computation in step 3", + "required_fix": "re-run with correct formula", + "evidence_ref": "file://line 42", + } + + +def _review_payload(submission_id: str, decision: str, **overrides) -> dict: + """Build a minimal review create payload with default empty findings/evidence.""" + body = { + "submission_id": submission_id, + "decision": decision, + "acceptance_evidence_refs": [], + "comment": None, + "findings": [], + } + body.update(overrides) + return body + + +# ── tests ── + + +class TestReviewDecisionValidation: + """Acceptance: API rejects unknown decision values.""" + + async def test_rejects_unknown_decision( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload(submission["id"], "approved") + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + + async def test_rejects_empty_decision( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload(submission["id"], "") # empty string + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + + +class TestRejectAndNeedsRevisionRequireFindings: + """Acceptance: reject / needs_revision require at least one finding.""" + + async def test_reject_without_findings_is_rejected( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload(submission["id"], REVIEW_DECISION_REJECT, findings=[]) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + assert "finding" in resp.text.lower() + + async def test_needs_revision_without_findings_is_rejected( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload(submission["id"], REVIEW_DECISION_NEEDS_REVISION, findings=[]) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + assert "finding" in resp.text.lower() + + +class TestAcceptRequiresEvidence: + """Acceptance: accept requires evidence refs and no blocking checker results.""" + + async def test_accept_without_evidence_is_rejected( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload(submission["id"], REVIEW_DECISION_ACCEPT, acceptance_evidence_refs=[]) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + assert "evidence" in resp.text.lower() + + async def test_accept_blocked_by_checker_result( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Accept must be rejected when a CheckerResult with blocks_review=True exists.""" + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + + # Seed a CheckerRun and a blocking CheckerResult with valid FK references + async with db_session.get_session_factory()() as session: + checker_run = CheckerRun( + id=str(uuid4()), + task_id=submission["task_id"], + submission_id=submission["id"], + submission_version=submission["version"], + trigger_source="manual", + status="completed", + routing_recommendation="not_evaluated", + outcome_source="manual", + triggered_by="reviewer-actor", + triggered_by_subject="reviewer-actor", + triggered_by_issuer="flow-test", + trigger_auth_source="dev_mock", + attempt_number=1, + is_current_for_submission=True, + locked_guide_version=submission["locked_guide_version"], + locked_post_submit_checker_policy_id=submission["locked_post_submit_checker_policy_id"], + locked_post_submit_checker_policy_version=submission["locked_post_submit_checker_policy_version"], + locked_post_submit_checker_policy_hash=submission["locked_post_submit_checker_policy_hash"], + locked_post_submit_checker_policy_body=submission["locked_post_submit_checker_policy_body"], + locked_review_policy_version=submission["locked_review_policy_version"], + locked_revision_policy_version=submission["locked_revision_policy_version"], + locked_payment_policy_version=submission["locked_payment_policy_version"], + package_hash=submission["package_hash"], + artifact_hash_manifest=submission.get("artifact_hash_manifest", []), + artifact_manifest_hash="sha256:test", + passed_count=0, + warning_count=0, + failed_count=1, + blocking_count=1, + ) + session.add(checker_run) + await session.flush() + + session.add(CheckerResult( + id=str(uuid4()), + checker_run_id=checker_run.id, + task_id=submission["task_id"], + submission_id=submission["id"], + checker_name="blocking-checker", + status="failed", + severity="high", + blocks_review=True, + message="blocked for testing", + )) + await session.commit() + + body = _review_payload( + submission["id"], + REVIEW_DECISION_ACCEPT, + acceptance_evidence_refs=["evidence/log.txt"], + ) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + assert "blocking" in resp.text.lower() + + +class TestAuditEventCreated: + """Acceptance: audit event records review decision with actor identity.""" + + async def test_review_creates_audit_event( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload( + submission["id"], + REVIEW_DECISION_NEEDS_REVISION, + findings=[_finding_payload("high")], + ) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 201, f"expected 201, got {resp.status_code}: {resp.text}" + review_json = resp.json() + assert review_json["decision"] == REVIEW_DECISION_NEEDS_REVISION + assert len(review_json["findings"]) == 1 + + # Verify audit event exists + async with db_session.get_session_factory()() as session: + audit = await session.scalar( + select(AuditEvent).where( + AuditEvent.entity_type == "review", + AuditEvent.entity_id == review_json["id"], + ) + ) + assert audit is not None, "no audit event found for created review" + assert audit.event_type == "review_decision_created" + assert audit.actor_id != "" + + +class TestSuccessfulDecisionFlows: + """End-to-end: valid decisions produce correct responses.""" + + async def test_reject_with_finding_succeeds( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload( + submission["id"], + REVIEW_DECISION_REJECT, + findings=[_finding_payload("critical"), _finding_payload("high")], + ) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 201, resp.text + data = resp.json() + assert data["decision"] == REVIEW_DECISION_REJECT + assert len(data["findings"]) == 2 + + async def test_accept_with_evidence_succeeds( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + body = _review_payload( + submission["id"], + REVIEW_DECISION_ACCEPT, + acceptance_evidence_refs=["evidence/checker-log.txt"], + ) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 201, resp.text + data = resp.json() + assert data["decision"] == REVIEW_DECISION_ACCEPT + assert len(data["acceptance_evidence_refs"]) == 1 + + async def test_query_reviews_for_submission( + self, review_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, + ) -> None: + set_dev_actor(monkeypatch, roles="project_manager,reviewer", subject="reviewer-actor") + submission = await _create_locked_submission(review_client, monkeypatch) + + # create two reviews + for _ in range(2): + body = _review_payload( + submission["id"], + REVIEW_DECISION_NEEDS_REVISION, + findings=[_finding_payload()], + ) + resp = await review_client.post("/api/v1/reviews", headers=auth_headers(), json=body) + assert resp.status_code == 201, resp.text + + # query + list_resp = await review_client.get( + f"/api/v1/reviews/{submission['id']}", headers=auth_headers(), + ) + assert list_resp.status_code == 200, list_resp.text + reviews = list_resp.json() + assert len(reviews) == 2