diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md index d51c2cb20..47b83fac7 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md @@ -2,9 +2,11 @@ ## Current status -PLAN4 end-to-end planning refresh is complete and internally reviewed from -current main `3479ee71`. No REV runtime module, table, route, action activation, -or product behavior exists. +`WS-REV-001-03A1` implements the hidden queue/admission persistence foundation. +PR #262 is reconciled with trusted main `2feaf47d`; ART retains migration 0050 +and REV owns its exact 0051 successor. The chunk adds no REV route, action +activation, checker hook, lease, Review, revision behavior, or contribution +behavior. AUTH `WS-XINT-003-02A` through `02D` are merged. REV now has stable policy lineage/mutation, complete unavailable action/principal registration, and typed @@ -40,6 +42,7 @@ REV does not own Project/Task/Submission/Checker/AUTH/ART/CON internals. ## Next step -Await human approval. Then refresh and implement only `WS-REV-001-03A1` -queue/admission-idempotency persistence from then-current main. Stop before -03A2. +Publish and review only the `WS-REV-001-03A1` PR. GitHub Actions must provide +the full-suite and repository-coverage proof. After human merge approval, stop; +`WS-REV-001-03A2` remains a separate explicit start and still depends on its +named CON policy-version FK target. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03A1-queue-admission-persistence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03A1-queue-admission-persistence.md index b9eccad0e..00a3d4b25 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03A1-queue-admission-persistence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03A1-queue-admission-persistence.md @@ -26,19 +26,25 @@ No expedited SLA. ## Allowed files -Freeze exact migration name from then-current main; expected scope: +Current main now has the single ART-owned head `0050_guide_source_v2`. After +rebasing PR #262, this chunk owns its exact successor +`0051_review_queue_foundation.py` and the following scope: ```text backend/app/modules/reviews/__init__.py backend/app/modules/reviews/models.py backend/app/modules/reviews/repository.py backend/app/modules/reviews/schemas.py -backend/alembic/versions/_review_queue_foundation.py +backend/app/db/models.py (metadata registration only) +backend/alembic/versions/0051_review_queue_foundation.py backend/tests/test_alembic.py backend/tests/test_review_queue_persistence.py backend/tests/conftest.py (schema fingerprint/fixture registration only) +backend/scripts/run_test_lanes.py (canonical lane registration only) +backend/tests/test_ci_test_lanes.py (lane registration assertion only) docs/architecture_data_model.md .agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/** +.agent-loop/merge-intents/WS-REV-001-03A1.json ``` The preimplementation refresh must replace `` and confirm exact metadata @@ -64,6 +70,11 @@ registration conventions before code. attempt without authorizing it or mutating upstream rows. - Database constraints permit at most one queue identity per Submission and reject cross-project/task/Submission lineage. +- A REV-owned PostgreSQL write-time guard rejects any mismatch among stored + project, task, Submission/version, and admitting CheckerRun identities. A + pending queue or committed admission requires that exact CheckerRun to be + completed, current for the Submission, and `allow_review`; no checker hook or + automatic admission is added. - No migration backfills historical submissions or fabricates CheckerRun/ART facts. Required foreign facts may remain unpopulated only in explicitly non-admitted setup shapes that cannot become pending. @@ -71,6 +82,11 @@ registration conventions before code. - Models contain no AUTH handle, token, grant query, ART locator/bytes, or CON state. - No router is registered and every REV lifecycle action remains unavailable. +- 03A1 cannot persist `leased` or an active-lease reference. Those shapes enter + only with the real REV-owned ReviewLease FK in 03A2. +- Admission idempotency enforces exact SHA-256 request digests, one replay + namespace/operation identity, pending-without-queue and committed-with-queue + shapes, and rejects conflicting reuse at the database boundary. ## Verification commands @@ -78,16 +94,20 @@ Freeze exact node IDs at start. Minimum proof: ```text cd backend && .venv/bin/alembic heads -cd backend && .venv/bin/pytest -q tests/test_alembic.py -k review_queue +cd backend && .venv/bin/pytest -q tests/test_alembic.py -k review_queue_foundation cd backend && .venv/bin/pytest -q tests/test_review_queue_persistence.py cd backend && .venv/bin/ruff check app/modules/reviews tests/test_review_queue_persistence.py tests/test_alembic.py -cd backend && .venv/bin/pytest --cov=app.modules.reviews.models --cov=app.modules.reviews.repository --cov-branch --cov-report=term-missing --cov-fail-under=90 -q tests/test_review_queue_persistence.py +cd backend && .venv/bin/pytest --cov=app.modules.reviews --cov-branch --cov-report=term-missing --cov-fail-under=90 -q tests/test_review_queue_persistence.py python3 scripts/check_stale_review_contracts.py python3 scripts/check_markdown_links.py git diff --check ``` GitHub Actions runs the full sharded suite and repository coverage floor. +Focused PostgreSQL proof must include mismatched task/project/Submission/checker +refusal, non-final/non-current/non-`allow_review` refusal, immutable lineage and +first-queued time, replay conflicts, no historical backfill, and populated +downgrade refusal followed by an empty safe round trip. ## Required reviewers diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-external-review-response.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-external-review-response.md new file mode 100644 index 000000000..ab3ab420b --- /dev/null +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-external-review-response.md @@ -0,0 +1,53 @@ +# External Review Response: WS-REV-001-03A1 + +## Comments addressed + +- CodeRabbit: queue updates could reopen a closed row or decrease routing or + lifecycle generations. The PostgreSQL guard now rejects both transitions, + with direct-SQL tests. +- CodeRabbit: `ReviewQueueEntryInput` exposed closed-state fields that every + database insert rejected. The insert schema now contains only admission + fields and the repository always inserts `pending` with no close metadata. +- CodeRabbit: the invalid `leased` assertion depended on PostgreSQL check + evaluation order. The assertion now accepts either relevant named check while + still proving that the state cannot persist. +- CodeRabbit nitpick: the empty migration round trip did not assert both + truncate-reject triggers. Both are now part of the exact expected state. +- GitHub Backend: every semantic lane failed inventory collection with + `missing_lane_modules:tests/test_review_queue_persistence.py`. The focused REV + module is now registered once in `task_lifecycle`, and the canonical lane + ownership assertion is updated. +- Trusted `main` advanced through ART PR #249 while the repair was under review. + ART retains its merged `0050_guide_source_v2`; REV is reconciled as the exact + `0051_review_queue_foundation` successor, with both shared test conflicts + resolved additively. + +## Comments deferred + +- CodeRabbit's 30.30 percent docstring warning is not a repository CI failure. + GitHub's authoritative docstring-coverage step passed on the reviewed head, + and the new runtime classes and methods already carry docstrings. No unrelated + test/migration-function documentation expansion was added. + +## Human decisions needed + +None. Every actionable finding was in scope and resolved without adding product +behavior or crossing REV ownership. + +## Commands rerun + +- Ruff over REV, migration tests, and lane-inventory files: PASS. +- `PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 pytest -q tests/test_ci_test_lanes.py`: + PASS, 33 tests. +- Isolated PostgreSQL `tests/test_review_queue_persistence.py` with complete + `app.modules.reviews` branch coverage and 90 percent floor: PASS. +- Isolated PostgreSQL `tests/test_alembic.py -k review_queue_foundation`: PASS. +- Reconciled ART+REV schema fingerprint and sole 0051 head: PASS. +- Two tests that needed a second lineage were updated for ART's merged unique + project/actor behavior; their isolated exact-node reruns pass. + +## Remaining risks + +Fresh GitHub semantic lanes/full coverage and CodeRabbit incremental review must +pass on the repaired commit. Human merge approval remains required, and this +repair does not start 03A2. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-internal-review-evidence.md new file mode 100644 index 000000000..88b3e2d12 --- /dev/null +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-internal-review-evidence.md @@ -0,0 +1,70 @@ +# Internal Review Evidence: WS-REV-001-03A1 + +## Candidate + +- Trusted base: `10720382cd9639f00f09578f772b97ab3afc358b` +- Reviewed implementation commit: `a5a778b4c1be2602d406fdf23c05bd8320f1c8cb` +- Scope: hidden REV queue/admission persistence, originally migration 0050 and + reconciled to migration 0051 after ART PR #249, focused tests, + data-model documentation, initiative status, and one merge intent +- Runtime exposure: none; no route, checker hook, lease, Review, revision, + contribution, AUTH, ART, or upstream mutation behavior is added + +## Reviewer results + +| Track | Result | Resolution | +|---|---:|---| +| Architecture | PASS | Scope includes metadata registration and merge intent; no ownership drift remains. | +| Senior engineering | PASS | Database-owned insert stamps and focused invariant tests resolved the original concerns. | +| QA/test | PASS | Fresh-id replay and all checker-admissibility branches are covered. | +| Product/ops | PASS | The change remains hidden persistence at the `allow_review` boundary. | +| Security/auth | PASS | Exact lineage, immutable identity, delete/truncate refusal, and downgrade safety are database-enforced. | +| Docs | PASS | Data-model wording distinguishes current persistence from later lease behavior. | +| CI integrity | PASS | No workflow or package-script change; no coverage gate was weakened. | +| Reuse/dedup | PASS with low risk | REV-specific repository patterns are appropriate; digest syntax remains locally duplicated to avoid importing an owner-specific AUTH or ART type. | +| Test delta | PASS | Direct constraint, trigger, replay, downgrade, and absence-of-lease proofs are present; no test was weakened or skipped. | + +## Findings repaired + +- Exact replay no longer depends on reuse of an internal row primary key. +- The contract now explicitly permits central metadata registration and the + required merge-intent artifact. +- Queue/admission creation timestamps and queue generations are stamped by + PostgreSQL, preventing caller-controlled queue age. +- Tests now cover non-completed, non-current, non-`allow_review`, checker + lineage, task/Submission lineage, and project mismatch refusal. +- Direct database tests isolate replay-key, operation, checker-run, digest, + state-shape, and committed-queue identity constraints. +- Admission-only and queue-only populated downgrade refusal are isolated and + prove that revision and protected rows survive the failed downgrade. +- The focused coverage command includes the complete new REV package. + +## Deterministic evidence + +- `backend/.venv/bin/alembic heads`: PASS on the original implementation; after + reconciliation with ART PR #249, the sole head is the REV successor + `0051_review_queue_foundation`. +- Isolated `tests/test_alembic.py -k review_queue_foundation`: PASS. +- Isolated `tests/test_review_queue_persistence.py`: PASS, 10 focused tests. +- Isolated `--cov=app.modules.reviews --cov-branch --cov-fail-under=90`: PASS. +- Ruff over the new REV package and focused tests: PASS. +- `python3 scripts/check_stale_review_contracts.py`: PASS. +- `python3 scripts/check_markdown_links.py`: PASS. +- `git diff --check`: PASS. + +The full test suite and repository-wide 78 percent coverage floor are reserved +for GitHub Actions, per repository operations guidance and the user instruction. + +## Remaining risks and gates + +- The same SHA-256 syntax exists in multiple bounded owner modules. Creating a + cross-owner shared type is not justified inside this REV chunk. +- PostgreSQL validates upstream lineage at the REV write boundary; the queue + row records that fact and does not constrain future upstream-owned mutation. +- GitHub Actions, CodeRabbit, and human review remain pending. +- Merge does not start 03A2. + +## Disposition + +PASS for PR publication after the evidence-only documentation delta receives +its final narrow review. No reviewer session may remain open at publication. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-pr-trust-bundle.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-pr-trust-bundle.md new file mode 100644 index 000000000..efafb7fa0 --- /dev/null +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03A1-pr-trust-bundle.md @@ -0,0 +1,106 @@ +# PR Trust Bundle: WS-REV-001-03A1 + +## Chunk + +`WS-REV-001-03A1` — Queue And Admission Persistence. + +## Goal + +Add the smallest hidden REV-owned persistence foundation for one exact +reviewable Submission queue identity and one idempotent admission operation. + +## Human-approved intent + +Start at a completed, current `allow_review` CheckerRun; consume the existing +Submission/version without changing upstream owners; stop before selection, +leases, Reviews, revisions, FinalAcceptance, or contributions. + +## What changed and why + +- Added `ReviewQueueEntry` and `ReviewAdmissionIdempotencyRecord` models, + schemas, and caller-transaction repository operations. +- Added REV Alembic revision 0051, following ART-owned 0050, with exact + lineage/admissibility guards, + immutable identity, replay constraints, delete/truncate protection, and + populated downgrade refusal. +- Registered the models and schema fingerprint, added focused PostgreSQL tests, + and clarified the data-model boundary. + +This separates stable queue/admission identity from the later concurrency and +policy-version concerns of REV-owned lease persistence. + +## Design chosen + +One immutable queue row references the existing project, Task, +Submission/version, and admitting CheckerRun. A separate pending-to-committed +idempotency row records replay identity and binds only to the exact matching +queue. PostgreSQL is the final invariant boundary; repository methods flush but +never commit the caller's transaction. + +## Alternatives rejected + +- No checker completion hook or automatic admission. +- No route, backlog read, reviewer selection, claim, lease, or active-lease + placeholder. +- No upstream row changes, AUTH lookup, ART locator/bytes, or CON state. +- No historical backfill or fabricated checker fact. +- No destructive downgrade after either protected table contains a row. + +## Scope and product behavior + +This PR changes only the reviewed 03A1 contract, REV initiative evidence/status, +REV models/repository/schemas, migration, metadata registration, focused tests, +data-model docs, and one schema-v2 merge intent. It exposes no product API or +review lifecycle action. + +## Acceptance criteria proof + +Database constraints and triggers prove one queue per Submission, exact +project/task/Submission/version/checker lineage, current completed +`allow_review`, server-owned queue age/generations, immutable identity, +open/preferred storage without lease shape, exact replay namespaces/digest, and +pending-to-committed admission binding. Direct tests cover every refusal path +and isolated downgrade refusal for each protected table. + +## Tests, test delta, and CI integrity + +Focused isolated PostgreSQL migration and persistence tests pass. Focused +branch coverage for the complete new REV package passes the 90 percent floor. +Ruff, the stale review-contract scan, changed Markdown links, Alembic one-head, +and diff integrity pass. No existing test, assertion, skip, workflow, package +script, global 78 percent baseline, or CI gate was weakened. GitHub Actions will +run the full suite and repository coverage. + +## Reviewer results and external review + +Architecture, senior engineering, QA/test, product/ops, security/auth, docs, +CI integrity, reuse/dedup, and test-delta tracks pass after resolving replay, +server-stamping, scope, coverage, constraint, and downgrade-test findings. +The first GitHub Backend run failed before test execution because the new test +module was absent from the closed semantic-lane inventory. The module is now +registered exactly once in `task_lifecycle`, and all 33 lane-integrity tests +pass. All three actionable CodeRabbit comments and its truncate-trigger nitpick +were resolved: queue lifecycle is monotonic, the insert schema is pending-only, +the lease-state assertion is constraint-order independent, and the migration +round trip asserts both truncate guards. CodeRabbit's docstring warning is not +an authoritative CI failure; GitHub's docstring gate passed. Fresh external +checks remain required on the repaired head. + +## Remaining risks and follow-up + +The queue preserves the checker admission fact at write time; it does not own +or constrain later upstream state. Digest syntax remains locally repeated +rather than introducing a cross-owner abstraction in this chunk. 03A2 remains +a separately approved successor and must not begin from this PR. + +## Human review focus + +Review exact cross-owner lineage, current `allow_review` enforcement, fresh-ID +replay semantics, server-stamped queue age, no lease/API shape, protected +downgrade behavior, and absence of AUTH/ART/CON ownership leakage. + +## Human merge ownership + +Only the user may approve and merge this specific PR. Do not merge while any +current-head GitHub or CodeRabbit check is pending or failed, or while an +actionable review comment remains unresolved. Merge does not authorize 03A2. diff --git a/.agent-loop/merge-intents/WS-REV-001-03A1.json b/.agent-loop/merge-intents/WS-REV-001-03A1.json new file mode 100644 index 000000000..0b88d7249 --- /dev/null +++ b/.agent-loop/merge-intents/WS-REV-001-03A1.json @@ -0,0 +1,9 @@ +{ + "chunk_id": "WS-REV-001-03A1", + "chunk_title": "Queue And Admission Persistence", + "initiative_id": "WS-REV-001", + "next_chunk_id": "WS-REV-001-03A2", + "next_chunk_title": "Lease And Preference Persistence", + "next_requires_explicit_start": true, + "schema_version": 2 +} diff --git a/backend/alembic/versions/0051_review_queue_foundation.py b/backend/alembic/versions/0051_review_queue_foundation.py new file mode 100644 index 000000000..72c2ff724 --- /dev/null +++ b/backend/alembic/versions/0051_review_queue_foundation.py @@ -0,0 +1,410 @@ +"""add hidden review queue and admission idempotency persistence + +Revision ID: 0051_review_queue_foundation +Revises: 0050_guide_source_v2 +Create Date: 2026-08-03 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + + +revision = "0051_review_queue_foundation" +down_revision = "0050_guide_source_v2" +branch_labels = depends_on = None + + +def _create_queue_table() -> None: + op.create_table( + "review_queue_entries", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("project_id", sa.String(36), nullable=False), + sa.Column("task_id", sa.String(36), nullable=False), + sa.Column("submission_id", sa.String(36), nullable=False), + sa.Column("submission_version", sa.Integer(), nullable=False), + sa.Column("admitting_checker_run_id", sa.String(36), nullable=False), + sa.Column("queue_state", sa.String(16), server_default="pending", nullable=False), + sa.Column("routing_mode", sa.String(16), nullable=False), + sa.Column("routing_reason", sa.String(32), nullable=False), + sa.Column( + "first_queued_at", + sa.DateTime(timezone=True), + server_default=sa.text("statement_timestamp()"), + nullable=False, + ), + sa.Column( + "available_since", + sa.DateTime(timezone=True), + server_default=sa.text("statement_timestamp()"), + nullable=False, + ), + sa.Column("preferred_reviewer_id", sa.String(36)), + sa.Column("preference_expires_at", sa.DateTime(timezone=True)), + sa.Column("closed_at", sa.DateTime(timezone=True)), + sa.Column("closed_reason", sa.String(32)), + sa.Column("routing_generation", sa.Integer(), server_default="1", nullable=False), + sa.Column("lifecycle_generation", sa.Integer(), server_default="1", nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("statement_timestamp()"), + nullable=False, + ), + sa.CheckConstraint( + "submission_version > 0", + name="ck_review_queue_entries_submission_version_positive", + ), + sa.CheckConstraint( + "queue_state in ('pending','closed')", + name="ck_review_queue_entries_queue_state", + ), + sa.CheckConstraint( + "routing_mode in ('open','preferred')", + name="ck_review_queue_entries_routing_mode", + ), + sa.CheckConstraint( + "routing_reason in ('first_submission','revision_return','admin_assignment')", + name="ck_review_queue_entries_routing_reason", + ), + sa.CheckConstraint( + "(routing_mode='open' and preferred_reviewer_id is null " + "and preference_expires_at is null) or " + "(routing_mode='preferred' and preferred_reviewer_id is not null " + "and preference_expires_at is not null " + "and preference_expires_at > first_queued_at)", + name="ck_review_queue_entries_routing_shape", + ), + sa.CheckConstraint( + "(queue_state='pending' and closed_at is null and closed_reason is null) or " + "(queue_state='closed' and closed_at is not null and " + "closed_reason in ('review_recorded','task_closed','admin_cancelled') " + "and closed_at >= first_queued_at)", + name="ck_review_queue_entries_lifecycle_shape", + ), + sa.CheckConstraint( + "available_since >= first_queued_at", + name="ck_review_queue_entries_availability_time", + ), + sa.CheckConstraint( + "routing_generation > 0 and lifecycle_generation > 0", + name="ck_review_queue_entries_generations_positive", + ), + sa.ForeignKeyConstraint( + ["project_id"], ["projects.id"], name="fk_review_queue_project" + ), + sa.ForeignKeyConstraint( + ["task_id"], + ["workstream_tasks.id"], + name="fk_review_queue_task", + ), + sa.ForeignKeyConstraint( + ["submission_id"], + ["submissions.id"], + name="fk_review_queue_submission", + ), + sa.ForeignKeyConstraint( + ["submission_id", "task_id", "submission_version"], + ["submissions.id", "submissions.task_id", "submissions.version"], + name="fk_review_queue_submission_lineage", + ), + sa.ForeignKeyConstraint( + ["admitting_checker_run_id"], + ["checker_runs.id"], + name="fk_review_queue_checker", + ), + sa.ForeignKeyConstraint( + ["preferred_reviewer_id"], + ["actor_profiles.id"], + name="fk_review_queue_preferred_reviewer", + ), + sa.PrimaryKeyConstraint("id", name="pk_review_queue_entries"), + sa.UniqueConstraint("submission_id", name="uq_review_queue_submission"), + sa.UniqueConstraint( + "id", + "project_id", + "task_id", + "submission_id", + "submission_version", + "admitting_checker_run_id", + name="uq_review_queue_admission_identity", + ), + ) + op.create_index( + "ix_review_queue_selection", + "review_queue_entries", + ["project_id", "queue_state", "routing_mode", "first_queued_at", "id"], + ) + op.create_index( + "ix_review_queue_preference", + "review_queue_entries", + ["preferred_reviewer_id", "queue_state", "preference_expires_at", "id"], + ) + + +def _create_admission_table() -> None: + op.create_table( + "review_admission_idempotency_records", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("idempotency_key", sa.Uuid(), nullable=False), + sa.Column("operation_id", sa.Uuid(), nullable=False), + sa.Column("request_digest", sa.String(71), nullable=False), + sa.Column("project_id", sa.String(36), nullable=False), + sa.Column("task_id", sa.String(36), nullable=False), + sa.Column("submission_id", sa.String(36), nullable=False), + sa.Column("submission_version", sa.Integer(), nullable=False), + sa.Column("admitting_checker_run_id", sa.String(36), nullable=False), + sa.Column("status", sa.String(16), server_default="pending", nullable=False), + sa.Column("review_queue_entry_id", sa.Uuid()), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("statement_timestamp()"), + nullable=False, + ), + sa.Column("committed_at", sa.DateTime(timezone=True)), + sa.CheckConstraint( + "submission_version > 0", + name="ck_review_admission_idempotency_records_submission_version_positive", + ), + sa.CheckConstraint( + "request_digest ~ '^sha256:[0-9a-f]{64}$'", + name="ck_review_admission_idempotency_records_request_digest", + ), + sa.CheckConstraint( + "status in ('pending','committed')", + name="ck_review_admission_idempotency_records_status", + ), + sa.CheckConstraint( + "(status='pending' and review_queue_entry_id is null and committed_at is null) or " + "(status='committed' and review_queue_entry_id is not null " + "and committed_at is not null)", + name="ck_review_admission_idempotency_records_state_shape", + ), + sa.ForeignKeyConstraint( + ["project_id"], + ["projects.id"], + name="fk_review_admission_project", + ), + sa.ForeignKeyConstraint( + ["task_id"], + ["workstream_tasks.id"], + name="fk_review_admission_task", + ), + sa.ForeignKeyConstraint( + ["submission_id"], + ["submissions.id"], + name="fk_review_admission_submission", + ), + sa.ForeignKeyConstraint( + ["submission_id", "task_id", "submission_version"], + ["submissions.id", "submissions.task_id", "submissions.version"], + name="fk_review_admission_submission_lineage", + ), + sa.ForeignKeyConstraint( + ["admitting_checker_run_id"], + ["checker_runs.id"], + name="fk_review_admission_checker", + ), + sa.ForeignKeyConstraint( + ["review_queue_entry_id"], + ["review_queue_entries.id"], + name="fk_review_admission_queue", + ), + sa.ForeignKeyConstraint( + [ + "review_queue_entry_id", + "project_id", + "task_id", + "submission_id", + "submission_version", + "admitting_checker_run_id", + ], + [ + "review_queue_entries.id", + "review_queue_entries.project_id", + "review_queue_entries.task_id", + "review_queue_entries.submission_id", + "review_queue_entries.submission_version", + "review_queue_entries.admitting_checker_run_id", + ], + name="fk_review_admission_committed_queue", + ), + sa.PrimaryKeyConstraint("id", name="pk_review_admission_idempotency_records"), + sa.UniqueConstraint( + "idempotency_key", name="uq_review_admission_replay_key" + ), + sa.UniqueConstraint("operation_id", name="uq_review_admission_operation"), + sa.UniqueConstraint( + "admitting_checker_run_id", name="uq_review_admission_checker_run" + ), + ) + op.create_index( + "ix_review_admission_submission", + "review_admission_idempotency_records", + ["submission_id", "status", "created_at", "id"], + ) + + +def _create_guards() -> None: + op.execute( + """ + create function guard_review_queue_entry() returns trigger language plpgsql as $$ + declare + task_project text; + checker_row checker_runs%rowtype; + begin + if tg_op='DELETE' then + raise exception 'review queue entries cannot be deleted' using errcode='55000'; + end if; + if tg_op='INSERT' then + if new.queue_state <> 'pending' then + raise exception 'review queue must begin pending' using errcode='23514'; + end if; + new.first_queued_at := statement_timestamp(); + new.available_since := new.first_queued_at; + new.routing_generation := 1; + new.lifecycle_generation := 1; + new.created_at := new.first_queued_at; + end if; + if tg_op='UPDATE' then + if (new.id,new.project_id,new.task_id,new.submission_id,new.submission_version, + new.admitting_checker_run_id,new.first_queued_at,new.created_at) + is distinct from + (old.id,old.project_id,old.task_id,old.submission_id,old.submission_version, + old.admitting_checker_run_id,old.first_queued_at,old.created_at) then + raise exception 'review queue identity is immutable' using errcode='55000'; + end if; + if old.queue_state='closed' and new.queue_state <> 'closed' then + raise exception 'closed review queue entries cannot reopen' using errcode='23514'; + end if; + if new.routing_generation < old.routing_generation + or new.lifecycle_generation < old.lifecycle_generation then + raise exception 'review queue generations cannot decrease' using errcode='23514'; + end if; + return new; + end if; + select project_id into task_project from workstream_tasks where id=new.task_id; + if task_project is null or task_project <> new.project_id then + raise exception 'review queue task project mismatch' using errcode='23514'; + end if; + select * into checker_row from checker_runs where id=new.admitting_checker_run_id; + if not found or checker_row.task_id <> new.task_id + or checker_row.submission_id <> new.submission_id + or checker_row.submission_version <> new.submission_version then + raise exception 'review queue checker lineage mismatch' using errcode='23514'; + end if; + if checker_row.status <> 'completed' + or checker_row.routing_recommendation <> 'allow_review' + or checker_row.is_current_for_submission is not true then + raise exception 'review queue checker is not admissible' using errcode='23514'; + end if; + return new; + end $$ + """ + ) + op.execute( + "create trigger review_queue_entries_guard before insert or update or delete " + "on review_queue_entries for each row execute function guard_review_queue_entry()" + ) + op.execute( + """ + create function guard_review_admission_record() returns trigger language plpgsql as $$ + declare + task_project text; + checker_row checker_runs%rowtype; + begin + if tg_op='DELETE' then + raise exception 'review admission records cannot be deleted' using errcode='55000'; + end if; + if tg_op='INSERT' and new.status <> 'pending' then + raise exception 'review admission must begin pending' using errcode='23514'; + end if; + if tg_op='INSERT' then + new.created_at := statement_timestamp(); + end if; + if tg_op='UPDATE' then + if (new.id,new.idempotency_key,new.operation_id,new.request_digest,new.project_id, + new.task_id,new.submission_id,new.submission_version, + new.admitting_checker_run_id,new.created_at) + is distinct from + (old.id,old.idempotency_key,old.operation_id,old.request_digest,old.project_id, + old.task_id,old.submission_id,old.submission_version, + old.admitting_checker_run_id,old.created_at) then + raise exception 'review admission identity is immutable' using errcode='55000'; + end if; + if old.status <> 'pending' or new.status <> 'committed' then + raise exception 'invalid review admission transition' using errcode='23514'; + end if; + end if; + select project_id into task_project from workstream_tasks where id=new.task_id; + if task_project is null or task_project <> new.project_id then + raise exception 'review admission task project mismatch' using errcode='23514'; + end if; + select * into checker_row from checker_runs where id=new.admitting_checker_run_id; + if not found or checker_row.task_id <> new.task_id + or checker_row.submission_id <> new.submission_id + or checker_row.submission_version <> new.submission_version then + raise exception 'review admission checker lineage mismatch' using errcode='23514'; + end if; + if new.status='committed' and ( + checker_row.status <> 'completed' + or checker_row.routing_recommendation <> 'allow_review' + or checker_row.is_current_for_submission is not true) then + raise exception 'review admission checker is not admissible' using errcode='23514'; + end if; + return new; + end $$ + """ + ) + op.execute( + "create trigger review_admission_records_guard before insert or update or delete " + "on review_admission_idempotency_records for each row " + "execute function guard_review_admission_record()" + ) + op.execute( + """ + create function reject_review_queue_foundation_truncate() returns trigger + language plpgsql as $$ + begin + raise exception 'review queue foundation cannot be truncated' using errcode='55000'; + end $$ + """ + ) + for table in ("review_queue_entries", "review_admission_idempotency_records"): + op.execute( + f"create trigger {table}_reject_truncate before truncate on {table} " + "execute function reject_review_queue_foundation_truncate()" + ) + + +def upgrade() -> None: + """Install empty hidden REV persistence without admitting historical work.""" + _create_queue_table() + _create_admission_table() + _create_guards() + + +def downgrade() -> None: + """Remove only an unused queue foundation; never discard review history.""" + bind = op.get_bind() + bind.execute(sa.text("lock table review_admission_idempotency_records in access exclusive mode")) + bind.execute(sa.text("lock table review_queue_entries in access exclusive mode")) + populated = bind.execute( + sa.text( + "select exists(select 1 from review_queue_entries) or " + "exists(select 1 from review_admission_idempotency_records)" + ) + ).scalar_one() + if populated: + raise RuntimeError("cannot downgrade populated review queue foundation") + for table in ("review_admission_idempotency_records", "review_queue_entries"): + op.execute(f"drop trigger {table}_reject_truncate on {table}") + op.execute("drop trigger review_admission_records_guard on review_admission_idempotency_records") + op.execute("drop function guard_review_admission_record()") + op.execute("drop trigger review_queue_entries_guard on review_queue_entries") + op.execute("drop function guard_review_queue_entry()") + op.execute("drop function reject_review_queue_foundation_truncate()") + op.drop_table("review_admission_idempotency_records") + op.drop_table("review_queue_entries") diff --git a/backend/app/db/models.py b/backend/app/db/models.py index 461e7ea98..a81928f4b 100644 --- a/backend/app/db/models.py +++ b/backend/app/db/models.py @@ -47,6 +47,10 @@ ReviewPolicy, SubmissionArtifactPolicy, ) +from app.modules.reviews.models import ( # noqa: F401 + ReviewAdmissionIdempotencyRecord, + ReviewQueueEntry, +) from app.modules.tasks.models import ( # noqa: F401 AuditEvent, EvidenceItem, diff --git a/backend/app/modules/reviews/__init__.py b/backend/app/modules/reviews/__init__.py new file mode 100644 index 000000000..edfabf466 --- /dev/null +++ b/backend/app/modules/reviews/__init__.py @@ -0,0 +1 @@ +"""Hidden review and revision lifecycle persistence.""" diff --git a/backend/app/modules/reviews/models.py b/backend/app/modules/reviews/models.py new file mode 100644 index 000000000..92f8b4ed4 --- /dev/null +++ b/backend/app/modules/reviews/models.py @@ -0,0 +1,216 @@ +"""SQLAlchemy persistence for the hidden review queue foundation.""" + +from __future__ import annotations + +from datetime import datetime +from uuid import UUID + +from sqlalchemy import ( + CheckConstraint, + DateTime, + ForeignKey, + ForeignKeyConstraint, + Index, + Integer, + String, + UniqueConstraint, + Uuid, + text, +) +from sqlalchemy.orm import Mapped, mapped_column + +from app.db.base import Base + + +class ReviewQueueEntry(Base): + """Mutable routing identity for exactly one reviewable Submission.""" + + __tablename__ = "review_queue_entries" + __table_args__ = ( + ForeignKeyConstraint( + ["submission_id", "task_id", "submission_version"], + ["submissions.id", "submissions.task_id", "submissions.version"], + name="fk_review_queue_submission_lineage", + ), + UniqueConstraint("submission_id", name="uq_review_queue_submission"), + UniqueConstraint( + "id", + "project_id", + "task_id", + "submission_id", + "submission_version", + "admitting_checker_run_id", + name="uq_review_queue_admission_identity", + ), + CheckConstraint("submission_version > 0", name="submission_version_positive"), + CheckConstraint("queue_state in ('pending','closed')", name="queue_state"), + CheckConstraint( + "routing_mode in ('open','preferred')", + name="routing_mode", + ), + CheckConstraint( + "routing_reason in ('first_submission','revision_return','admin_assignment')", + name="routing_reason", + ), + CheckConstraint( + "(routing_mode='open' and preferred_reviewer_id is null " + "and preference_expires_at is null) or " + "(routing_mode='preferred' and preferred_reviewer_id is not null " + "and preference_expires_at is not null " + "and preference_expires_at > first_queued_at)", + name="routing_shape", + ), + CheckConstraint( + "(queue_state='pending' and closed_at is null and closed_reason is null) or " + "(queue_state='closed' and closed_at is not null and " + "closed_reason in ('review_recorded','task_closed','admin_cancelled') " + "and closed_at >= first_queued_at)", + name="lifecycle_shape", + ), + CheckConstraint( + "available_since >= first_queued_at", + name="availability_time", + ), + CheckConstraint( + "routing_generation > 0 and lifecycle_generation > 0", + name="generations_positive", + ), + Index( + "ix_review_queue_selection", + "project_id", + "queue_state", + "routing_mode", + "first_queued_at", + "id", + ), + Index( + "ix_review_queue_preference", + "preferred_reviewer_id", + "queue_state", + "preference_expires_at", + "id", + ), + ) + + id: Mapped[UUID] = mapped_column(Uuid(), primary_key=True) + project_id: Mapped[str] = mapped_column( + ForeignKey("projects.id", name="fk_review_queue_project"), nullable=False + ) + task_id: Mapped[str] = mapped_column( + ForeignKey("workstream_tasks.id", name="fk_review_queue_task"), nullable=False + ) + submission_id: Mapped[str] = mapped_column( + ForeignKey("submissions.id", name="fk_review_queue_submission"), nullable=False + ) + submission_version: Mapped[int] = mapped_column(Integer, nullable=False) + admitting_checker_run_id: Mapped[str] = mapped_column( + ForeignKey("checker_runs.id", name="fk_review_queue_checker"), nullable=False + ) + queue_state: Mapped[str] = mapped_column( + String(16), nullable=False, server_default=text("'pending'") + ) + routing_mode: Mapped[str] = mapped_column(String(16), nullable=False) + routing_reason: Mapped[str] = mapped_column(String(32), nullable=False) + first_queued_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=text("statement_timestamp()") + ) + available_since: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=text("statement_timestamp()") + ) + preferred_reviewer_id: Mapped[str | None] = mapped_column( + ForeignKey("actor_profiles.id", name="fk_review_queue_preferred_reviewer") + ) + preference_expires_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + closed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + closed_reason: Mapped[str | None] = mapped_column(String(32)) + routing_generation: Mapped[int] = mapped_column( + Integer, nullable=False, server_default=text("1") + ) + lifecycle_generation: Mapped[int] = mapped_column( + Integer, nullable=False, server_default=text("1") + ) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=text("statement_timestamp()") + ) + + +class ReviewAdmissionIdempotencyRecord(Base): + """Reservation and replay identity for one future queue admission.""" + + __tablename__ = "review_admission_idempotency_records" + __table_args__ = ( + ForeignKeyConstraint( + ["submission_id", "task_id", "submission_version"], + ["submissions.id", "submissions.task_id", "submissions.version"], + name="fk_review_admission_submission_lineage", + ), + ForeignKeyConstraint( + [ + "review_queue_entry_id", + "project_id", + "task_id", + "submission_id", + "submission_version", + "admitting_checker_run_id", + ], + [ + "review_queue_entries.id", + "review_queue_entries.project_id", + "review_queue_entries.task_id", + "review_queue_entries.submission_id", + "review_queue_entries.submission_version", + "review_queue_entries.admitting_checker_run_id", + ], + name="fk_review_admission_committed_queue", + ), + UniqueConstraint("idempotency_key", name="uq_review_admission_replay_key"), + UniqueConstraint("operation_id", name="uq_review_admission_operation"), + UniqueConstraint("admitting_checker_run_id", name="uq_review_admission_checker_run"), + CheckConstraint("submission_version > 0", name="submission_version_positive"), + CheckConstraint( + "request_digest ~ '^sha256:[0-9a-f]{64}$'", + name="request_digest", + ), + CheckConstraint("status in ('pending','committed')", name="status"), + CheckConstraint( + "(status='pending' and review_queue_entry_id is null and committed_at is null) or " + "(status='committed' and review_queue_entry_id is not null " + "and committed_at is not null)", + name="state_shape", + ), + Index( + "ix_review_admission_submission", + "submission_id", + "status", + "created_at", + "id", + ), + ) + + id: Mapped[UUID] = mapped_column(Uuid(), primary_key=True) + idempotency_key: Mapped[UUID] = mapped_column(Uuid(), nullable=False) + operation_id: Mapped[UUID] = mapped_column(Uuid(), nullable=False) + request_digest: Mapped[str] = mapped_column(String(71), nullable=False) + project_id: Mapped[str] = mapped_column( + ForeignKey("projects.id", name="fk_review_admission_project"), nullable=False + ) + task_id: Mapped[str] = mapped_column( + ForeignKey("workstream_tasks.id", name="fk_review_admission_task"), nullable=False + ) + submission_id: Mapped[str] = mapped_column( + ForeignKey("submissions.id", name="fk_review_admission_submission"), nullable=False + ) + submission_version: Mapped[int] = mapped_column(Integer, nullable=False) + admitting_checker_run_id: Mapped[str] = mapped_column( + ForeignKey("checker_runs.id", name="fk_review_admission_checker"), nullable=False + ) + status: Mapped[str] = mapped_column( + String(16), nullable=False, server_default=text("'pending'") + ) + review_queue_entry_id: Mapped[UUID | None] = mapped_column( + Uuid(), ForeignKey("review_queue_entries.id", name="fk_review_admission_queue") + ) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=text("statement_timestamp()") + ) + committed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) diff --git a/backend/app/modules/reviews/repository.py b/backend/app/modules/reviews/repository.py new file mode 100644 index 000000000..1718b8662 --- /dev/null +++ b/backend/app/modules/reviews/repository.py @@ -0,0 +1,144 @@ +"""Caller-transaction persistence for review queue identity and admission replay.""" + +from __future__ import annotations + +from dataclasses import dataclass +from uuid import UUID + +from sqlalchemy import or_, select, text, update +from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.ext.asyncio import AsyncSession + +from app.modules.reviews.models import ( + ReviewAdmissionIdempotencyRecord, + ReviewQueueEntry, +) +from app.modules.reviews.schemas import ( + ReviewAdmissionReservationInput, + ReviewQueueEntryInput, +) + + +class ReviewAdmissionIdempotencyConflict(RuntimeError): + """A replay identity was reused for different immutable admission facts.""" + + +@dataclass(frozen=True, slots=True) +class ReviewAdmissionReservation: + """Locked reservation returned to a future caller-owned admission command.""" + + created: bool + record: ReviewAdmissionIdempotencyRecord + + +class ReviewQueueRepository: + """Persist queue identities without authorizing or selecting review work.""" + + def __init__(self, session: AsyncSession) -> None: + self._session = session + + async def add_queue_entry(self, value: ReviewQueueEntryInput) -> ReviewQueueEntry: + """Flush one database-validated queue identity without committing.""" + record = ReviewQueueEntry( + id=value.id, + project_id=value.project_id, + task_id=value.task_id, + submission_id=value.submission_id, + submission_version=value.submission_version, + admitting_checker_run_id=value.admitting_checker_run_id, + queue_state="pending", + routing_mode=value.routing_mode.value, + routing_reason=value.routing_reason.value, + preferred_reviewer_id=value.preferred_reviewer_id, + preference_expires_at=value.preference_expires_at, + ) + self._session.add(record) + await self._session.flush() + return record + + async def reserve_admission( + self, + value: ReviewAdmissionReservationInput, + ) -> ReviewAdmissionReservation: + """Reserve one replay/checker identity and lock every possible conflict.""" + created_id = await self._session.scalar( + insert(ReviewAdmissionIdempotencyRecord) + .values( + id=value.id, + idempotency_key=value.idempotency_key, + operation_id=value.operation_id, + request_digest=value.request_digest, + project_id=value.project_id, + task_id=value.task_id, + submission_id=value.submission_id, + submission_version=value.submission_version, + admitting_checker_run_id=value.admitting_checker_run_id, + ) + .on_conflict_do_nothing() + .returning(ReviewAdmissionIdempotencyRecord.id) + ) + await self._session.flush() + records = tuple( + ( + await self._session.scalars( + select(ReviewAdmissionIdempotencyRecord) + .where( + or_( + ReviewAdmissionIdempotencyRecord.idempotency_key + == value.idempotency_key, + ReviewAdmissionIdempotencyRecord.operation_id + == value.operation_id, + ReviewAdmissionIdempotencyRecord.admitting_checker_run_id + == value.admitting_checker_run_id, + ) + ) + .order_by(ReviewAdmissionIdempotencyRecord.id) + .with_for_update() + .execution_options(populate_existing=True) + ) + ).all() + ) + if len(records) != 1 or not self._matches_reservation(records[0], value): + raise ReviewAdmissionIdempotencyConflict("review_admission_idempotency_conflict") + return ReviewAdmissionReservation(created=created_id is not None, record=records[0]) + + async def commit_admission( + self, + *, + reservation_id: UUID, + queue_entry_id: UUID, + ) -> ReviewAdmissionIdempotencyRecord: + """Bind a pending reservation to its exact queue row without committing.""" + record = await self._session.scalar( + update(ReviewAdmissionIdempotencyRecord) + .where( + ReviewAdmissionIdempotencyRecord.id == reservation_id, + ReviewAdmissionIdempotencyRecord.status == "pending", + ) + .values( + status="committed", + review_queue_entry_id=queue_entry_id, + committed_at=text("statement_timestamp()"), + ) + .returning(ReviewAdmissionIdempotencyRecord) + ) + if record is None: + raise ReviewAdmissionIdempotencyConflict("review_admission_not_pending") + await self._session.flush() + return record + + @staticmethod + def _matches_reservation( + record: ReviewAdmissionIdempotencyRecord, + value: ReviewAdmissionReservationInput, + ) -> bool: + return ( + record.idempotency_key == value.idempotency_key + and record.operation_id == value.operation_id + and record.request_digest == value.request_digest + and record.project_id == value.project_id + and record.task_id == value.task_id + and record.submission_id == value.submission_id + and record.submission_version == value.submission_version + and record.admitting_checker_run_id == value.admitting_checker_run_id + ) diff --git a/backend/app/modules/reviews/schemas.py b/backend/app/modules/reviews/schemas.py new file mode 100644 index 000000000..940c8aab6 --- /dev/null +++ b/backend/app/modules/reviews/schemas.py @@ -0,0 +1,57 @@ +"""Closed persistence inputs for the hidden review queue foundation.""" + +from __future__ import annotations + +from datetime import datetime +from enum import StrEnum +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, Field + + +class ReviewRoutingMode(StrEnum): + """Stored routing shapes; selection behavior is implemented later.""" + + OPEN = "open" + PREFERRED = "preferred" + + +class ReviewRoutingReason(StrEnum): + """Closed reasons for the initial stored routing shape.""" + + FIRST_SUBMISSION = "first_submission" + REVISION_RETURN = "revision_return" + ADMIN_ASSIGNMENT = "admin_assignment" + + +class ReviewQueueEntryInput(BaseModel): + """Exact values for one queue identity; this input grants no authority.""" + + model_config = ConfigDict(frozen=True, extra="forbid") + + id: UUID + project_id: str = Field(min_length=36, max_length=36) + task_id: str = Field(min_length=36, max_length=36) + submission_id: str = Field(min_length=36, max_length=36) + submission_version: int = Field(gt=0) + admitting_checker_run_id: str = Field(min_length=36, max_length=36) + routing_mode: ReviewRoutingMode + routing_reason: ReviewRoutingReason + preferred_reviewer_id: str | None = Field(default=None, min_length=36, max_length=36) + preference_expires_at: datetime | None = None + + +class ReviewAdmissionReservationInput(BaseModel): + """One idempotent admission reservation without lifecycle authorization.""" + + model_config = ConfigDict(frozen=True, extra="forbid") + + id: UUID + idempotency_key: UUID + operation_id: UUID + request_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") + project_id: str = Field(min_length=36, max_length=36) + task_id: str = Field(min_length=36, max_length=36) + submission_id: str = Field(min_length=36, max_length=36) + submission_version: int = Field(gt=0) + admitting_checker_run_id: str = Field(min_length=36, max_length=36) diff --git a/backend/scripts/run_test_lanes.py b/backend/scripts/run_test_lanes.py index aaf293879..0c2f445d8 100644 --- a/backend/scripts/run_test_lanes.py +++ b/backend/scripts/run_test_lanes.py @@ -154,6 +154,7 @@ class TestLane: "task_lifecycle", ( "tests/test_checkers.py", + "tests/test_review_queue_persistence.py", "tests/test_tasks.py", ), ), diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 1ac7796c6..10eec036c 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -21,7 +21,7 @@ from scripts.run_isolated_tests import LOOPBACK, NAME_RE, ROLE_RE DDL_LOCK_DIRECTORY = Path("/tmp") -EXPECTED_PUBLIC_SCHEMA_SHA256 = "ecbfc3fa0eda089f226918e902502807a89558f0c7e6e59c4dac05feadc57b34" +EXPECTED_PUBLIC_SCHEMA_SHA256 = "80a21fac3c65f252f4b0aaa9ec037a2327fb20ff941e6d4c6a840db80e66ff1c" PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", "alembic_version", @@ -81,6 +81,8 @@ "projects", "review_policies", "revision_policies", + "review_admission_idempotency_records", + "review_queue_entries", "submission_artifact_policies", "submissions", "task_assignments", @@ -98,6 +100,8 @@ "project_create_idempotency_records", "project_role_grants", "project_role_qualification_snapshots", + "review_admission_idempotency_records", + "review_queue_entries", "review_policies", "revision_policies", ) diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 500a04f99..7fc62be2a 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -73,7 +73,7 @@ snapshot_existing_service_rows, ) -HEAD_REVISION = "0050_guide_source_v2" +HEAD_REVISION = "0051_review_queue_foundation" pytestmark = pytest.mark.postgres_schema_contract @@ -108,6 +108,42 @@ def _alembic_config() -> Config: return config +async def _review_queue_foundation_state(database_url: str) -> dict[str, object]: + engine = create_async_engine(database_url) + try: + async with engine.connect() as connection: + revision = await connection.scalar(text("select version_num from alembic_version")) + queue_count = await connection.scalar(text("select count(*) from review_queue_entries")) + admission_count = await connection.scalar( + text("select count(*) from review_admission_idempotency_records") + ) + triggers = set( + ( + await connection.execute( + text( + "select tgname from pg_trigger where tgrelid in " + "('review_queue_entries'::regclass, " + "'review_admission_idempotency_records'::regclass) " + "and not tgisinternal" + ) + ) + ).scalars() + ) + return { + "revision": str(revision), + "queue_count": int(queue_count or 0), + "admission_count": int(admission_count or 0), + "queue_guard": "review_queue_entries_guard" in triggers, + "admission_guard": "review_admission_records_guard" in triggers, + "queue_truncate_guard": "review_queue_entries_reject_truncate" in triggers, + "admission_truncate_guard": ( + "review_admission_idempotency_records_reject_truncate" in triggers + ), + } + finally: + await engine.dispose() + + def test_service_identity_migration_contract_is_frozen_from_application_modules() -> None: backend_root = Path(__file__).resolve().parents[1] revision_source = (backend_root / "alembic/versions/0023_service_actor_identity.py").read_text( @@ -212,6 +248,29 @@ async def _project_setup_run_check_constraint_names(database_url: str) -> set[st await engine.dispose() +def test_0051_review_queue_foundation_empty_round_trip( + isolated_database_env: str, + migration_lock, +) -> None: + """0051 creates no queue history and reverses only while still unused.""" + config = _alembic_config() + with migration_lock(): + command.downgrade(config, "0050_guide_source_v2") + command.upgrade(config, "0051_review_queue_foundation") + state = asyncio.run(_review_queue_foundation_state(isolated_database_env)) + assert state == { + "revision": "0051_review_queue_foundation", + "queue_count": 0, + "admission_count": 0, + "queue_guard": True, + "admission_guard": True, + "queue_truncate_guard": True, + "admission_truncate_guard": True, + } + command.downgrade(config, "0050_guide_source_v2") + command.upgrade(config, "head") + + def test_0034_project_role_issue_evidence_exact_safe_round_trip( isolated_database_env: str, migration_lock, diff --git a/backend/tests/test_ci_test_lanes.py b/backend/tests/test_ci_test_lanes.py index 75f888a58..3c62b1db9 100644 --- a/backend/tests/test_ci_test_lanes.py +++ b/backend/tests/test_ci_test_lanes.py @@ -43,6 +43,7 @@ def test_measured_hotspots_have_explicit_semantic_owners() -> None: assert modules_by_lane["project_lifecycle"] == {"tests/test_projects.py"} assert modules_by_lane["task_lifecycle"] == { "tests/test_checkers.py", + "tests/test_review_queue_persistence.py", "tests/test_tasks.py", } assert { diff --git a/backend/tests/test_review_queue_persistence.py b/backend/tests/test_review_queue_persistence.py new file mode 100644 index 000000000..a52a3d8c2 --- /dev/null +++ b/backend/tests/test_review_queue_persistence.py @@ -0,0 +1,628 @@ +"""Focused PostgreSQL proof for the hidden REV queue foundation.""" + +from __future__ import annotations + +import asyncio +from collections.abc import AsyncIterator, Iterator +from datetime import UTC, datetime, timedelta +from pathlib import Path +from uuid import uuid4 + +from alembic import command +from alembic.config import Config +from httpx import ASGITransport, AsyncClient +import pytest +from sqlalchemy import select, text +from sqlalchemy.exc import DBAPIError, IntegrityError + +from app.core.config import get_settings +from app.db import session as db_session +from app.db.base import Base +from app.main import create_app +from app.modules.checkers.models import CheckerRun +from app.modules.reviews.models import ( + ReviewAdmissionIdempotencyRecord, + ReviewQueueEntry, +) +from app.modules.reviews.repository import ( + ReviewAdmissionIdempotencyConflict, + ReviewQueueRepository, +) +from app.modules.reviews.schemas import ( + ReviewAdmissionReservationInput, + ReviewQueueEntryInput, + ReviewRoutingMode, + ReviewRoutingReason, +) +from app.modules.tasks.models import Submission +from project_create_fixtures import grant_system_project_manager, insert_historical_project +from tests.test_checkers import get_submission_and_automatic_pre_review_run +from tests.test_tasks import ( + auth_headers, + complete_submission_payload, + create_active_project, + create_started_task, + set_dev_actor, +) + + +@pytest.fixture +def review_database_env( + monkeypatch: pytest.MonkeyPatch, + clean_postgres_database: str, +) -> Iterator[str]: + """Bind one test to the runner-owned migrated PostgreSQL database.""" + monkeypatch.setenv("WORKSTREAM_DATABASE_URL", clean_postgres_database) + monkeypatch.setenv("WORKSTREAM_CELERY_TASK_ALWAYS_EAGER", "true") + monkeypatch.setenv( + "WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", + "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", + ) + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + get_settings.cache_clear() + try: + yield clean_postgres_database + finally: + get_settings.cache_clear() + + +@pytest.fixture +async def review_client(review_database_env: str) -> AsyncIterator[AsyncClient]: + """Return an API client used only to create canonical upstream test facts.""" + app = create_app() + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + admission = await client.get("/api/v1/auth/me", headers=auth_headers()) + assert admission.status_code == 200, admission.text + async with db_session.get_session_factory()() as session: + await grant_system_project_manager( + session, + issuer="flow-test", + subject="project-manager-subject", + ) + await session.commit() + yield client + + +async def _reviewable_lineage( + client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> tuple[dict, dict, dict]: + project = await create_active_project(client) + task = await create_started_task( + client, + project["id"], + monkeypatch, + subject="review-worker-two", + ) + submission_response = await client.post( + f"/api/v1/tasks/{task['id']}/submissions", + headers=auth_headers(), + json=complete_submission_payload(), + ) + assert submission_response.status_code == 201, submission_response.text + submission = submission_response.json() + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + _, checker = await get_submission_and_automatic_pre_review_run(client, submission["id"]) + assert checker["status"] == "completed" + assert checker["routing_recommendation"] == "allow_review" + return project, task, submission | {"checker_run_id": checker["id"]} + + +async def _additional_reviewable_submission( + client: AsyncClient, + project: dict, + monkeypatch: pytest.MonkeyPatch, +) -> tuple[dict, dict]: + """Create another exact task/submission/checker lineage in one project.""" + task = await create_started_task(client, project["id"], monkeypatch) + submission_response = await client.post( + f"/api/v1/tasks/{task['id']}/submissions", + headers=auth_headers(), + json=complete_submission_payload(), + ) + assert submission_response.status_code == 201, submission_response.text + submission = submission_response.json() + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + _, checker = await get_submission_and_automatic_pre_review_run(client, submission["id"]) + assert checker["status"] == "completed" + assert checker["routing_recommendation"] == "allow_review" + return task, submission | {"checker_run_id": checker["id"]} + + +def _queue_input(project: dict, task: dict, submission: dict) -> ReviewQueueEntryInput: + return ReviewQueueEntryInput( + id=uuid4(), + project_id=project["id"], + task_id=task["id"], + submission_id=submission["id"], + submission_version=submission["version"], + admitting_checker_run_id=submission["checker_run_id"], + routing_mode=ReviewRoutingMode.OPEN, + routing_reason=ReviewRoutingReason.FIRST_SUBMISSION, + ) + + +def _reservation_input( + project: dict, + task: dict, + submission: dict, +) -> ReviewAdmissionReservationInput: + return ReviewAdmissionReservationInput( + id=uuid4(), + idempotency_key=uuid4(), + operation_id=uuid4(), + request_digest="sha256:" + "a" * 64, + project_id=project["id"], + task_id=task["id"], + submission_id=submission["id"], + submission_version=submission["version"], + admitting_checker_run_id=submission["checker_run_id"], + ) + + +def test_review_models_are_registered_without_routes() -> None: + """Alembic sees the tables while the application exposes no REV router.""" + assert "review_queue_entries" in Base.metadata.tables + assert "review_admission_idempotency_records" in Base.metadata.tables + assert "active_lease_id" not in Base.metadata.tables["review_queue_entries"].columns + assert "review_lease_id" not in Base.metadata.tables["review_queue_entries"].columns + assert { + "queue_state", + "closed_at", + "closed_reason", + }.isdisjoint(ReviewQueueEntryInput.model_fields) + route_paths = {getattr(route, "path", None) for route in create_app().routes} + assert not any(path and path.startswith("/api/v1/reviews") for path in route_paths) + + +@pytest.mark.asyncio +async def test_repository_reserves_and_commits_exact_queue_identity( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + queue_input = _queue_input(project, task, submission) + reservation_input = _reservation_input(project, task, submission) + + async with db_session.get_session_factory()() as session: + repository = ReviewQueueRepository(session) + reservation = await repository.reserve_admission(reservation_input) + assert reservation.created is True + assert reservation.record.status == "pending" + queue = await repository.add_queue_entry(queue_input) + committed = await repository.commit_admission( + reservation_id=reservation.record.id, + queue_entry_id=queue.id, + ) + assert committed.status == "committed" + await session.commit() + + async with db_session.get_session_factory()() as session: + queue = await session.get(ReviewQueueEntry, queue_input.id) + admission = await session.get(ReviewAdmissionIdempotencyRecord, reservation_input.id) + assert queue is not None + assert queue.queue_state == "pending" + assert queue.routing_generation == queue.lifecycle_generation == 1 + assert admission is not None + assert admission.review_queue_entry_id == queue_input.id + + +@pytest.mark.asyncio +async def test_repository_exact_replay_and_conflict( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + value = _reservation_input(project, task, submission) + async with db_session.get_session_factory()() as session: + repository = ReviewQueueRepository(session) + first = await repository.reserve_admission(value) + replay = await repository.reserve_admission(value.model_copy(update={"id": uuid4()})) + assert first.created is True + assert replay.created is False + assert replay.record.id == first.record.id + conflict = value.model_copy(update={"request_digest": "sha256:" + "b" * 64}) + with pytest.raises(ReviewAdmissionIdempotencyConflict): + await repository.reserve_admission(conflict) + await session.rollback() + + +@pytest.mark.asyncio +async def test_database_rejects_invalid_admission_state_and_commit( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + reservation_value = _reservation_input(project, task, submission) + async with db_session.get_session_factory()() as session: + invalid_digest = ReviewAdmissionIdempotencyRecord( + **reservation_value.model_dump(exclude={"request_digest"}), + request_digest="not-a-sha256-digest", + ) + session.add(invalid_digest) + with pytest.raises(IntegrityError, match="request_digest"): + await session.flush() + await session.rollback() + + async with db_session.get_session_factory()() as session: + invalid_committed = ReviewAdmissionIdempotencyRecord( + **reservation_value.model_copy(update={"id": uuid4()}).model_dump(), + status="committed", + committed_at=datetime.now(UTC), + ) + session.add(invalid_committed) + with pytest.raises(DBAPIError, match="review admission must begin pending"): + await session.flush() + await session.rollback() + + queue_value = _queue_input(project, task, submission) + async with db_session.get_session_factory()() as session: + repository = ReviewQueueRepository(session) + reservation = await repository.reserve_admission(reservation_value) + queue = await repository.add_queue_entry(queue_value) + checker = await session.get(CheckerRun, submission["checker_run_id"]) + assert checker is not None + checker.is_current_for_submission = False + await session.flush() + with pytest.raises(DBAPIError, match="review admission checker is not admissible"): + await repository.commit_admission( + reservation_id=reservation.record.id, + queue_entry_id=queue.id, + ) + await session.rollback() + + +@pytest.mark.asyncio +async def test_database_enforces_admission_replay_and_queue_identity_constraints( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + other_task, other_submission = await _additional_reviewable_submission( + review_client, project, monkeypatch + ) + base = _reservation_input(project, task, submission) + other = _reservation_input(project, other_task, other_submission) + async with db_session.get_session_factory()() as session: + session.add(ReviewAdmissionIdempotencyRecord(**base.model_dump())) + await session.commit() + + pending_lineage_failures = ( + ( + base.model_copy( + update={ + "id": uuid4(), + "idempotency_key": uuid4(), + "operation_id": uuid4(), + "project_id": str(uuid4()), + } + ), + "review admission task project mismatch", + ), + ( + base.model_copy( + update={ + "id": uuid4(), + "idempotency_key": uuid4(), + "operation_id": uuid4(), + "admitting_checker_run_id": other_submission["checker_run_id"], + } + ), + "review admission checker lineage mismatch", + ), + ) + for invalid_lineage, error_message in pending_lineage_failures: + async with db_session.get_session_factory()() as session: + session.add(ReviewAdmissionIdempotencyRecord(**invalid_lineage.model_dump())) + with pytest.raises(DBAPIError, match=error_message): + await session.flush() + await session.rollback() + + duplicates = ( + ( + other.model_copy(update={"id": uuid4(), "idempotency_key": base.idempotency_key}), + "uq_review_admission_replay_key", + ), + ( + other.model_copy(update={"id": uuid4(), "operation_id": base.operation_id}), + "uq_review_admission_operation", + ), + ( + base.model_copy( + update={"id": uuid4(), "idempotency_key": uuid4(), "operation_id": uuid4()} + ), + "uq_review_admission_checker_run", + ), + ) + for duplicate, constraint_name in duplicates: + async with db_session.get_session_factory()() as session: + session.add(ReviewAdmissionIdempotencyRecord(**duplicate.model_dump())) + with pytest.raises(IntegrityError, match=constraint_name): + await session.flush() + await session.rollback() + + base_queue = _queue_input(project, task, submission) + other_queue = _queue_input(project, other_task, other_submission) + async with db_session.get_session_factory()() as session: + repository = ReviewQueueRepository(session) + await repository.add_queue_entry(base_queue) + await repository.add_queue_entry(other_queue) + await session.commit() + + async with db_session.get_session_factory()() as session: + with pytest.raises(IntegrityError, match="fk_review_admission_committed_queue"): + await session.execute( + text( + "update review_admission_idempotency_records " + "set status='committed', review_queue_entry_id=:queue_id, " + "committed_at=statement_timestamp() where id=:id" + ), + {"queue_id": other_queue.id, "id": base.id}, + ) + await session.rollback() + + +@pytest.mark.asyncio +async def test_database_rejects_non_admissible_checker_and_project_mismatch( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + for field, invalid_value in ( + ("status", "running"), + ("routing_recommendation", "needs_revision"), + ("is_current_for_submission", False), + ): + async with db_session.get_session_factory()() as session: + checker = await session.get(CheckerRun, submission["checker_run_id"]) + assert checker is not None + setattr(checker, field, invalid_value) + await session.flush() + session.add( + ReviewQueueEntry( + **_queue_input(project, task, submission) + .model_copy(update={"id": uuid4()}) + .model_dump() + ) + ) + with pytest.raises(DBAPIError, match="review queue checker is not admissible"): + await session.flush() + await session.rollback() + + other_task, other_submission = await _additional_reviewable_submission( + review_client, project, monkeypatch + ) + task_mismatch = _queue_input(project, task, submission).model_copy( + update={"id": uuid4(), "task_id": other_task["id"]} + ) + async with db_session.get_session_factory()() as session: + session.add(ReviewQueueEntry(**task_mismatch.model_dump())) + with pytest.raises(DBAPIError, match="review queue checker lineage mismatch"): + await session.flush() + await session.rollback() + + checker_mismatch = _queue_input(project, task, submission).model_copy( + update={ + "id": uuid4(), + "admitting_checker_run_id": other_submission["checker_run_id"], + } + ) + async with db_session.get_session_factory()() as session: + session.add(ReviewQueueEntry(**checker_mismatch.model_dump())) + with pytest.raises(DBAPIError, match="review queue checker lineage mismatch"): + await session.flush() + await session.rollback() + + other_project_id = str(uuid4()) + async with db_session.get_session_factory()() as session: + await insert_historical_project( + session, + project_id=other_project_id, + name="Other review project", + slug=f"other-review-{other_project_id[:8]}", + ) + await session.commit() + mismatched = _queue_input(project, task, submission).model_copy( + update={"id": uuid4(), "project_id": other_project_id} + ) + async with db_session.get_session_factory()() as session: + session.add(ReviewQueueEntry(**mismatched.model_dump())) + with pytest.raises(DBAPIError, match="review queue task project mismatch"): + await session.flush() + await session.rollback() + + +@pytest.mark.asyncio +async def test_database_enforces_routing_uniqueness_and_immutable_lineage( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + value = _queue_input(project, task, submission) + async with db_session.get_session_factory()() as session: + queue = ReviewQueueEntry(**value.model_dump()) + session.add(queue) + await session.commit() + + async with db_session.get_session_factory()() as session: + duplicate = ReviewQueueEntry(**value.model_copy(update={"id": uuid4()}).model_dump()) + session.add(duplicate) + with pytest.raises(IntegrityError): + await session.flush() + await session.rollback() + + async with db_session.get_session_factory()() as session: + await session.execute( + text("update review_queue_entries set routing_generation=2 where id=:id"), + {"id": value.id}, + ) + await session.commit() + async with db_session.get_session_factory()() as session: + with pytest.raises(DBAPIError, match="review queue generations cannot decrease"): + await session.execute( + text("update review_queue_entries set routing_generation=1 where id=:id"), + {"id": value.id}, + ) + await session.rollback() + + async with db_session.get_session_factory()() as session: + await session.execute( + text("update review_queue_entries set lifecycle_generation=2 where id=:id"), + {"id": value.id}, + ) + await session.commit() + async with db_session.get_session_factory()() as session: + with pytest.raises(DBAPIError, match="review queue generations cannot decrease"): + await session.execute( + text("update review_queue_entries set lifecycle_generation=1 where id=:id"), + {"id": value.id}, + ) + await session.rollback() + + async with db_session.get_session_factory()() as session: + await session.execute( + text( + "update review_queue_entries set queue_state='closed', " + "closed_at=statement_timestamp(), closed_reason='admin_cancelled', " + "lifecycle_generation=2 where id=:id" + ), + {"id": value.id}, + ) + await session.commit() + async with db_session.get_session_factory()() as session: + with pytest.raises(DBAPIError, match="closed review queue entries cannot reopen"): + await session.execute( + text( + "update review_queue_entries set queue_state='pending', " + "closed_at=null, closed_reason=null, lifecycle_generation=3 where id=:id" + ), + {"id": value.id}, + ) + await session.rollback() + + async with db_session.get_session_factory()() as session: + with pytest.raises(DBAPIError, match="review queue identity is immutable"): + await session.execute( + text( + "update review_queue_entries set first_queued_at=:changed where id=:id" + ), + {"changed": datetime.now(UTC) + timedelta(seconds=5), "id": value.id}, + ) + await session.rollback() + with pytest.raises(DBAPIError, match="review queue identity is immutable"): + await session.execute( + text("update review_queue_entries set project_id=:changed where id=:id"), + {"changed": str(uuid4()), "id": value.id}, + ) + await session.rollback() + with pytest.raises(DBAPIError, match="review queue entries cannot be deleted"): + await session.execute( + text("delete from review_queue_entries where id=:id"), + {"id": value.id}, + ) + await session.rollback() + + +@pytest.mark.asyncio +async def test_preferred_shape_is_storage_only_and_lease_shape_is_impossible( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + async with db_session.get_session_factory()() as session: + contributor_id = await session.scalar( + select(Submission.contributor_id).where(Submission.id == submission["id"]) + ) + assert contributor_id is not None + preferred = _queue_input(project, task, submission).model_copy( + update={ + "routing_mode": ReviewRoutingMode.PREFERRED, + "routing_reason": ReviewRoutingReason.REVISION_RETURN, + "preferred_reviewer_id": contributor_id, + "preference_expires_at": datetime.now(UTC) + timedelta(hours=1), + } + ) + await ReviewQueueRepository(session).add_queue_entry(preferred) + await session.commit() + + async with db_session.get_session_factory()() as session: + with pytest.raises( + IntegrityError, + match=r"ck_review_queue_entries_(queue_state|lifecycle_shape)", + ): + await session.execute( + text("update review_queue_entries set queue_state='leased' where id=:id"), + {"id": preferred.id}, + ) + await session.rollback() + + +@pytest.mark.postgres_schema_contract +@pytest.mark.asyncio +async def test_populated_review_queue_foundation_refuses_downgrade( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, + migration_lock, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + reservation_value = _reservation_input(project, task, submission) + async with db_session.get_session_factory()() as session: + await ReviewQueueRepository(session).reserve_admission(reservation_value) + await session.commit() + await db_session.dispose_engine() + + backend_root = Path(__file__).resolve().parents[1] + config = Config(str(backend_root / "alembic.ini")) + config.set_main_option("script_location", str(backend_root / "alembic")) + + def downgrade() -> None: + with migration_lock(): + command.downgrade(config, "0050_guide_source_v2") + + with pytest.raises(RuntimeError, match="cannot downgrade populated review queue foundation"): + await asyncio.to_thread(downgrade) + + async with db_session.get_session_factory()() as session: + assert await session.scalar(text("select version_num from alembic_version")) == ( + "0051_review_queue_foundation" + ) + assert await session.scalar( + select(ReviewAdmissionIdempotencyRecord.id).where( + ReviewAdmissionIdempotencyRecord.id == reservation_value.id + ) + ) == reservation_value.id + + +@pytest.mark.postgres_schema_contract +@pytest.mark.asyncio +async def test_populated_review_queue_row_alone_refuses_downgrade( + review_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, + migration_lock, +) -> None: + project, task, submission = await _reviewable_lineage(review_client, monkeypatch) + queue_value = _queue_input(project, task, submission) + async with db_session.get_session_factory()() as session: + await ReviewQueueRepository(session).add_queue_entry(queue_value) + await session.commit() + await db_session.dispose_engine() + + backend_root = Path(__file__).resolve().parents[1] + config = Config(str(backend_root / "alembic.ini")) + config.set_main_option("script_location", str(backend_root / "alembic")) + + def downgrade() -> None: + with migration_lock(): + command.downgrade(config, "0050_guide_source_v2") + + with pytest.raises(RuntimeError, match="cannot downgrade populated review queue foundation"): + await asyncio.to_thread(downgrade) + + async with db_session.get_session_factory()() as session: + assert await session.scalar(text("select version_num from alembic_version")) == ( + "0051_review_queue_foundation" + ) + assert await session.get(ReviewQueueEntry, queue_value.id) is not None diff --git a/docs/architecture_data_model.md b/docs/architecture_data_model.md index 4295d3860..ef0781426 100644 --- a/docs/architecture_data_model.md +++ b/docs/architecture_data_model.md @@ -1510,17 +1510,29 @@ For v0.1, the current `CheckerRun` is the readiness proof. If any submitted arti ## ReviewQueueEntry And ReviewLease -`ReviewQueueEntry` immutably anchors one exact finalized Submission and its -current successful admitting CheckerRun. Mutable routing state carries -preferred/open/closed lifecycle, original queue age, and current preference. -The reviewer current-work API returns an active lease, one server-selected +`ReviewQueueEntry` immutably anchors one exact finalized Submission/version, +Task, project, and its current successful `allow_review` CheckerRun. The 03A1 +foundation persists only `pending` and `closed` queue state plus open/preferred +routing metadata; it exposes no route, selection behavior, or lease shape. +PostgreSQL validates the cross-owner lineage and checker admissibility when the +queue identity is written. The immutable queue row preserves that admission +fact if an upstream current-checker pointer changes later; it does not constrain +later mutations of upstream-owned rows. + +`ReviewAdmissionIdempotencyRecord` reserves one exact admission operation and +SHA-256 request digest. It may begin pending without a queue, but can become +committed only when it references the matching queue identity and the same +completed, current `allow_review` CheckerRun. This is replay persistence, not an +automatic checker hook or authorization decision. + +The later reviewer current-work API returns an active lease, one server-selected offer, or none; it never exposes the full backlog. `ReviewLease` is the permanent identity of one claim attempt. It stores the canonical human reviewer ActorProfile ID, queue/Submission lineage, database lease times, disposition, and the independently frozen reviewer ContributionPolicyVersion. PostgreSQL enforces one active lease per reviewer and -queue entry. +queue entry. `ReviewLease` is not part of the 03A1 foundation. `ReviewPacketManifest` is an immutable REV semantic projection over the exact lease, Submission, admitting CheckerRun/results, stamped context, response