From 9df24d2ea420b4d764dbc8198e37f9409f6ef369 Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 02:56:54 +0100 Subject: [PATCH 1/9] plan(rev): define canonical packet persistence and live guide identity --- .../initiatives/WS-REV-001/WS-REV-001-03B.md | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 .commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md new file mode 100644 index 000000000..a0bc16c47 --- /dev/null +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -0,0 +1,209 @@ +# REV-03B — Immutable normalized reviewer packet persistence + +- Initiative: `WS-REV-001` +- Durable disposition: `Planned` +- Risk: L1 (bounded schema, immutable evidence and owner contract correction). +- Intended merge outcome: REV persists one exact metadata packet per ReviewLease with normalized guide members; complete REV-04A Review storage is next. No claim, resolver, byte access or acceptance is activated. + +## Intent + +Continue the approved packet -> complete Review -> shared FinalAcceptance storage +sequence, so automated acceptance can become the first complete runtime path +without fabricating a Review. Build on merged ART-07A1 and the existing queue, +lease, Submission, checker and activated guide owners. + +Current main `7754703f` has no packet persistence. Discovery also found that +ART-07A1's `guide_binding_id` targets the retired extraction binding table. +`retired_guide_material_write_guard` rejects every write to that table. The live +upload and guide manifest use `GuideSourceArtifactIngest.id`, exposed as +`ingest_id` by the PROJECTS public guide document contract. Replace the mistaken +field with that live identity in this affected scope; do not activate the old +table, add an alias, or delete retained data. Existing extraction evidence stays +read-only. The complete packet remains metadata-only. + +## Bounded change + +### Allowed implementation and proof files + +- `backend/app/modules/artifacts/api/review_packet.py`: replace guide_binding_id with ingest_id, exact current owner vocabulary; no second contract. +- `backend/tests/artifacts/test_review_packet_contract.py`: update identity assertions and reject the removed field explicitly; retain all required proof. +- `backend/app/modules/reviews/packet_models.py`: normalized packet header and guide item models. +- `backend/app/modules/reviews/packet_schemas.py`: strict internal persistence input and detached stored result. +- `backend/app/modules/reviews/packet_repository.py`: caller-transaction persistence/replay and project-qualified stored metadata read. +- `backend/app/modules/projects/models.py`: correct the ingest model's stale not-yet-bound docstring only. +- `backend/app/db/models.py`: register the two REV models. +- `backend/alembic/versions/0012_review_packet.py`: additive tables, exact foreign keys, immutable/completeness guards, guide-ingest immutability, safe SQL name resolution. +- `backend/alembic/env.py`: accept predecessor and current migration head. +- `backend/tests/reviews/packet/__init__.py` +- `backend/tests/reviews/packet/support.py` +- `backend/tests/reviews/packet/test_storage.py` +- `backend/tests/reviews/packet/test_repository.py` +- `backend/tests/reviews/packet/test_migration.py` +- `backend/tests/conftest.py`: exact new resettable/guarded tables and measured schema fingerprint; no weaker validation. +- `backend/tests/test_alembic.py`: exact revision graph. +- `backend/scripts/test_lane_catalogue.py` and `backend/tests/test_ci_lane_catalogue.py`: additive registration in existing task lanes. +- `backend/scripts/behavior_ownership.py`, `backend/tests/test_behavior_ownership.py`, `.ci/behavior-ownership/partition.v1.json`: exact three-module REV addition and neighbor-rejection proof. + +### Allowed current documentation + +- This record; `.commitrail/INDEX.md`. +- `.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md`: document the current corrected guide identity and link this repair, preserving its original delivery history. +- `.commitrail/initiatives/WS-ART-001/OVERVIEW.md` +- `.commitrail/initiatives/WS-REV-001/OVERVIEW.md` +- `.commitrail/initiatives/WS-ARCH-001/OVERVIEW.md` +- `.commitrail/initiatives/WS-AUTH-001/OVERVIEW.md` +- `.commitrail/initiatives/WS-POL-003/OVERVIEW.md` +- `.commitrail/initiatives/WS-CON-001/OVERVIEW.md` +- `.commitrail/initiatives/WS-AUTH-003/OVERVIEW.md` +- `.commitrail/initiatives/WS-ARCH-001/planning/PLAN.md` +- `.commitrail/initiatives/WS-ARCH-001/planning/CHUNK_MAP.md` +- `.commitrail/initiatives/WS-AUTH-001/planning/PLAN.md` +- `.commitrail/initiatives/WS-AUTH-001/planning/CHUNK_MAP.md` +- `.commitrail/initiatives/WS-POL-003/planning/PLAN.md` +- `.commitrail/initiatives/WS-POL-003/planning/CHUNK_MAP.md` +- `.commitrail/initiatives/WS-ARCH-001/planning/chunks/WS-ARCH-001-04E-canonical-allow-review.md` +- `README.md`, `docs/roadmap_status.md`, `docs/spec_review_lifecycle.md`, `docs/spec_artifact_storage_service.md`, `docs/architecture_data_model.md`, `docs/engineering/authorization_activation_custody.md`. +- Local ignored roadmap exports only if present. + +### Prohibited changes + +No public route, ART resolver or materializer, provider I/O, extraction revival, +AUTH action/permission activation, claim/queue routing implementation, Review, +FinalAcceptance, CON effect, worker or acceptance trigger. No baseline rewrite, +retained-data deletion, compatibility field/alias, generic manifest framework, +JSON member set, workflow/timeout/coverage gate change or test suppression. + +## Design + +### Exact metadata and two normalized tables + +`ReviewGuideMember` uses `ingest_id`, `source_item_id`, `item_order`, +`logical_role='guide_source_original'` and the existing guide media type. The +logical role describes a packet member; it does not assert a row in the retired +binding table. The existing header request retains all eleven scope fields. + +`review_packet_manifests` columns: + +- `id` (UUIDv7), `created_at` (PostgreSQL clock). +- `review_lease_id` (unique), `review_queue_entry_id`. +- The eleven ART request fields: `project_id`, `task_id`, `submission_id`, + `submission_version`, `checker_run_id`, `result_id`, `guide_id`, `guide_version`, + `source_snapshot_id`, `project_setup_run_id`, `setup_generation`. +- `submission_binding_id`, `submission_logical_role`, `submission_media_type`. + +Exactly one required ZIP is represented by non-null header fields; no separate +one-to-one item table. All IDs/references use native PostgreSQL UUID, with native +Python UUID at new typed boundaries. Existing owner string representations stay +at existing boundaries. `result_id` is the aggregate CheckerRun.result_id. + +`review_packet_guide_items` columns: `packet_id`, `source_item_id` (composite +primary key), `ingest_id`, `item_order`, `logical_role`, `media_type`. +Unique packet/ingest and packet/order. No surrogate ID for this natural member +key. Closed roles/media types, bounded nonblank guide version, positive versions, +nonnegative item order. No hash, size, provider/content/replica identity, receipt, +capability, raw policy, arbitrary metadata or source body in either table. + +### Database custody + +Restrictive FKs bind header to lease, queue, exact Submission/version, checker +run/result, guide/project/version, snapshot/project/guide, setup/snapshot/generation +and Submission binding; item rows reference header, source item and live ingest. +Additional canonical checks reconcile: + +- Lease and queue exactly match the full project/task/Submission lineage and + queue admitting run; checker is a completed allow-review source with matching + aggregate result. Storage does not replace live claim/currentness authority. +- Submission's locked guide version/snapshot and original ZIP binding match; + binding belongs to the exact Submission/project and original role/media. +- Guide activation custody identifies the exact setup run/generation, including + a superseded historical guide; never select current/latest guide or setup. +- Each guide item matches its ingest/source-item, declared order and media, + and exact header snapshot. The complete declared guide set is present in both + directions, with 1..100 members. Missing/extra/swapped valid foreign members + fail independently of malformed-ID guards. + +Use deferred final-state checks for the atomic header/member insertion. No +persisted draft/sealed state or seal workflow is necessary: immutable header and +items, exact-set validation and unique natural identities prevent later append, +mutation or deletion. Parent absence must fail explicitly, including concurrent +parent visibility; no missing-row branch silently returns success. Protect +TRUNCATE as well as row mutations. PostgreSQL assigns creation time; callers +cannot forge it. Future replay is a read of the retained exact packet, not a +fresh claim or clock reset. + +The live ingest writer `ArtifactRepository` already returns an identical row or +rejects conflicting prepared bytes; no production update/delete consumer exists. +Add unconditional update/delete/truncate protection for these immutable ingest +facts so retained packets cannot change meaning through a referenced source. +Do not add a check-then-update race dependent on whether a packet currently +exists. Snapshot items, Submission bindings, terminal checker evidence and +activated guide custody already have immutable owner protections. Preserve +mutable availability/status transitions that are outside semantic membership. + +All new SQL functions use `SET search_path=pg_catalog,public,pg_temp`, fully +qualified protected relations/function calls and unambiguous argument names. +Temp-table names must not influence constraints. No privileged bypass fixture. + +### Repository and proof boundary + +`ReviewPacketRepository.store(lease_id, membership)` locks the exact +project-qualified REV lease before selecting/writing that lease's packet. It +allocates a UUIDv7 only for a new packet, inserts header and all guide rows, and +flushes without committing. Deferred checks stay within the caller transaction. +An identical retry returns the stored identity; any membership difference raises +one bounded conflict. Same-lease concurrency serializes through the lease row; +failed/rolled-back writes leave no packet or members. It acquires no AUTH, +project or contributor lock and cannot grant authority. + +`read(project_id, lease_id)` returns only normalized stored metadata as detached +strict facts, ordered by source item order; qualify ownership before lookup. +`ReviewPacketStored` contains `packet_id`, `review_lease_id`, +`review_queue_entry_id`, `created_at`, and the canonical ART `membership` value. +A missing or foreign scope returns no packet. Authorization is a future caller's +responsibility; no endpoint or runtime composition exposes this repository. + +## Acceptance criteria + +- Correct the unusable ART field; removed `guide_binding_id` is rejected, never + accepted as an alias. Existing strictness/privacy/shape proof is retained. +- Real PostgreSQL positive control creates a packet from current activated guide, + admitted Submission and completed checker facts, then compares all header and + nested values with their canonical owners. +- SQL rejects independently mismatched valid owners, wrong numeric version, + result/setup substitutions, omitted/extra guide members, wrong ZIP/media/order, + all immutable mutations, late appends, parent absence and source-ingest changes. +- Save/replay, changed replay, rollback, independent project lookup, concurrent + same-lease store and retained historical read preserve identities and evidence. +- Temp-shadow probe cannot change validation. Mutation probes removing the + specific membership/immutability guards fail the intended test assertions after + a valid control, not fixture setup. +- Upgrade from 0011 preserves populated Submission/checker/guide/queue/lease facts + and creates empty packet tables. Fresh schema, metadata parity, identifier + inventory, exact reset schema and existing owner tests pass. +- Current navigation advances to complete REV-04A source storage; resolver, + canonical claim and byte authority remain future work. Both acceptance branches + continue to share one operation, without synthetic Review/actor. + +## Risk and review routing + +L1: architecture/reuse, security, QA/test-delta, documentation/product operations, +CI integrity. Pre-implementation review must check live ingest ownership, +reference immutability, declared-set completeness, concurrent insert proof and +real-fixture feasibility. Required human focus: exact retained lineage and no +runtime permission from metadata. One coherent schema/repository change may +exceed the usual 500-line preference because migration, direct-SQL failure proof +and current navigation are inseparable; no extra lifecycle is included. + +## Evidence + +Reuse `tests/tasks/post_submit_routing/support.completed_source` for actual +activated guide/admitted Submission/completed checker owners; use the existing +REV queue/lease repository with a real human actor and that Submission's stamped +published contribution policy. Guide ingests and declared source rows come from +the current unified guide fixture. Do not disable extraction write guards or +fabricate checker success/authorization receipts. These fixtures prove storage, +not a live claim API or ART resolver. Run focused tests with the canonical +isolated PostgreSQL runner, strict contract tests, migration/identifier/ownership +and lane inventory checks; run Ruff, module boundaries, Commitrail, links and +stale wording. Full hosted nine-lane and real API checks remain required, with +no skipped/deselected nodes. No spreadsheet exports are currently present. From 904bdd55b08e3c80ea7efacef147b21eea27c13f Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 03:03:13 +0100 Subject: [PATCH 2/9] plan(rev): reconcile packet authority identity and custody proof --- .../initiatives/WS-REV-001/WS-REV-001-03B.md | 73 +++++++++++++++++-- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index a0bc16c47..00bb60cfc 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -27,9 +27,10 @@ read-only. The complete packet remains metadata-only. - `backend/app/modules/artifacts/api/review_packet.py`: replace guide_binding_id with ingest_id, exact current owner vocabulary; no second contract. - `backend/tests/artifacts/test_review_packet_contract.py`: update identity assertions and reject the removed field explicitly; retain all required proof. -- `backend/app/modules/reviews/packet_models.py`: normalized packet header and guide item models. -- `backend/app/modules/reviews/packet_schemas.py`: strict internal persistence input and detached stored result. -- `backend/app/modules/reviews/packet_repository.py`: caller-transaction persistence/replay and project-qualified stored metadata read. +- `backend/app/modules/reviews/packet/__init__.py`: package marker. +- `backend/app/modules/reviews/packet/models.py`: normalized packet header and guide item models. +- `backend/app/modules/reviews/packet/schemas.py`: strict internal persistence input and detached stored result. +- `backend/app/modules/reviews/packet/repository.py`: caller-transaction persistence/replay and project-qualified stored metadata read. - `backend/app/modules/projects/models.py`: correct the ingest model's stale not-yet-bound docstring only. - `backend/app/db/models.py`: register the two REV models. - `backend/alembic/versions/0012_review_packet.py`: additive tables, exact foreign keys, immutable/completeness guards, guide-ingest immutability, safe SQL name resolution. @@ -86,6 +87,8 @@ binding table. The existing header request retains all eleven scope fields. - `id` (UUIDv7), `created_at` (PostgreSQL clock). - `review_lease_id` (unique), `review_queue_entry_id`. +- `packet_manifest_generation` equals the immutable lease attempt generation; + `packet_manifest_digest` is the semantic digest defined below. - The eleven ART request fields: `project_id`, `task_id`, `submission_id`, `submission_version`, `checker_run_id`, `result_id`, `guide_id`, `guide_version`, `source_snapshot_id`, `project_setup_run_id`, `setup_generation`. @@ -100,9 +103,18 @@ at existing boundaries. `result_id` is the aggregate CheckerRun.result_id. primary key), `ingest_id`, `item_order`, `logical_role`, `media_type`. Unique packet/ingest and packet/order. No surrogate ID for this natural member key. Closed roles/media types, bounded nonblank guide version, positive versions, -nonnegative item order. No hash, size, provider/content/replica identity, receipt, +nonnegative item order. No artifact content hash, size, provider/content/replica identity, receipt, capability, raw policy, arbitrary metadata or source body in either table. +The semantic manifest digest uses existing `canonical_json_hash` over the exact +ART membership JSON: request, submission and ordered guide members, with native +UUIDs rendered as strings. It excludes packet UUID, lease identity, generation +and creation time, so future claim can compute it before allocating a lease. +Those independent identities remain explicitly bound by AUTH. SQL recomputes +the same digest using the existing canonical JSON function and pgcrypto digest; +caller-supplied false digests fail at commit. The positive Python/SQL parity +proof includes all fields and a changed ordered member. No second serializer. + ### Database custody Restrictive FKs bind header to lease, queue, exact Submission/version, checker @@ -111,7 +123,10 @@ and Submission binding; item rows reference header, source item and live ingest. Additional canonical checks reconcile: - Lease and queue exactly match the full project/task/Submission lineage and - queue admitting run; checker is a completed allow-review source with matching + queue admitting run. New packets require active lease, leased queue and exact + active-lease pointer while holding lease then queue locks. Later closure and + historical reads remain permitted; replay does not create a new packet. + Checker is a completed allow-review source with matching aggregate result. Storage does not replace live claim/currentness authority. - Submission's locked guide version/snapshot and original ZIP binding match; binding belongs to the exact Submission/project and original role/media. @@ -121,6 +136,13 @@ Additional canonical checks reconcile: and exact header snapshot. The complete declared guide set is present in both directions, with 1..100 members. Missing/extra/swapped valid foreign members fail independently of malformed-ID guards. +- Every ingest has a committed guide upload for the exact project/source item: + object-confirmed put, exact content/replica/namespace and ingest byte identity, + plus either its document-stored operation receipt or exact observed-confirmed + observation receipt. Use the same immutable predicates as the live ART guide + manifest. Do not freeze mutable replica availability or integrity status; + future byte resolution must recheck those. Prepared ingest alone is insufficient. + No current-draft/latest-snapshot resolver is reused for historical membership. Use deferred final-state checks for the atomic header/member insertion. No persisted draft/sealed state or seal workflow is necessary: immutable header and @@ -134,7 +156,8 @@ fresh claim or clock reset. The live ingest writer `ArtifactRepository` already returns an identical row or rejects conflicting prepared bytes; no production update/delete consumer exists. Add unconditional update/delete/truncate protection for these immutable ingest -facts so retained packets cannot change meaning through a referenced source. +facts using existing `reject_artifact_fact_mutation`, so retained packets cannot +change meaning through a referenced source. Do not add a check-then-update race dependent on whether a packet currently exists. Snapshot items, Submission bindings, terminal checker evidence and activated guide custody already have immutable owner protections. Preserve @@ -147,7 +170,10 @@ Temp-table names must not influence constraints. No privileged bypass fixture. ### Repository and proof boundary `ReviewPacketRepository.store(lease_id, membership)` locks the exact -project-qualified REV lease before selecting/writing that lease's packet. It +project-qualified REV lease, then its owning queue, before selecting/writing that +lease's packet (the existing lease-then-queue order). New insertion requires +active/leased/exact-pointer facts under those locks; replay remains possible +after closure. It allocates a UUIDv7 only for a new packet, inserts header and all guide rows, and flushes without committing. Deferred checks stay within the caller transaction. An identical retry returns the stored identity; any membership difference raises @@ -158,7 +184,8 @@ project or contributor lock and cannot grant authority. `read(project_id, lease_id)` returns only normalized stored metadata as detached strict facts, ordered by source item order; qualify ownership before lookup. `ReviewPacketStored` contains `packet_id`, `review_lease_id`, -`review_queue_entry_id`, `created_at`, and the canonical ART `membership` value. +`review_queue_entry_id`, `packet_manifest_generation`, `packet_manifest_digest`, +`created_at`, and the canonical ART `membership` value. A missing or foreign scope returns no packet. Authorization is a future caller's responsibility; no endpoint or runtime composition exposes this repository. @@ -207,3 +234,33 @@ isolated PostgreSQL runner, strict contract tests, migration/identifier/ownershi and lane inventory checks; run Ruff, module boundaries, Commitrail, links and stale wording. Full hosted nine-lane and real API checks remain required, with no skipped/deselected nodes. No spreadsheet exports are currently present. + +### Named future proof inventory + +These are implementation obligations, not claims of executed tests: + +- `test_packet_matches_canonical_owners_and_digest`: every header/nested owner + value and Python/SQL digest parity, including changed ordered membership. +- `test_packet_rejects_coherent_owner_substitutions`: independently valid foreign + project, task, Submission/version, run/result, activated setup/generation, ZIP. +- `test_packet_rejects_null_source_fields`: each nonnullable source independently. +- `test_packet_creation_time_is_database_owned`: supplied NULL/past/future times. +- `test_packet_requires_active_exact_lease`: terminal and sibling lease insertions. +- `test_packet_requires_committed_guide_upload`: prepared-only ingest cannot pass. +- `test_packet_requires_complete_canonical_guide_set`: omission, extra, swapped + ingest/source, order and media; valid control before each negative commit. +- `test_packet_and_ingest_facts_are_immutable`: direct UPDATE/DELETE/TRUNCATE for + header, members and ingest, and late insert into a completed packet. +- `test_packet_deferred_failure_rolls_back`: no retained header/member on failure. +- `test_packet_same_lease_concurrency`: exact and conflicting concurrent writes. +- `test_packet_creation_serializes_with_lease_closure`: cannot create post-close. +- `test_packet_read_conceals_foreign_project`: real stored foreign identity. +- `test_packet_retains_superseded_guide_lineage`: real successor activation, then + historical read and identical replay without selecting the successor. +- `test_packet_shadow_tables_cannot_change_custody`: temporary name shadowing. +- `test_packet_upgrade_preserves_existing_owners`: populated predecessor upgrade. + +Mutation probes target the relevant predicate after valid fixture setup: digest, +lease status/pointer, committed-upload receipt, exact set, source identity and +immutability. Each must reach the negative assertion and fail there when its +guard is removed; harness mismatch/setup failure is not discriminating proof. From abe78c0d10ea3a660795e52ee456effe9a4afbca Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 03:04:59 +0100 Subject: [PATCH 3/9] plan(rev): cover observed guide upload custody --- .commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index 00bb60cfc..25ab4b5b6 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -246,7 +246,11 @@ These are implementation obligations, not claims of executed tests: - `test_packet_rejects_null_source_fields`: each nonnullable source independently. - `test_packet_creation_time_is_database_owned`: supplied NULL/past/future times. - `test_packet_requires_active_exact_lease`: terminal and sibling lease insertions. -- `test_packet_requires_committed_guide_upload`: prepared-only ingest cannot pass. +- `test_packet_requires_committed_guide_upload`: prepared-only ingest cannot pass; + independently crossed content, replica, namespace, receipt, ingest byte and + media identities fail after a valid operation-receipt control. +- `test_packet_accepts_observed_confirmed_upload`: genuine observed-confirmed + recovery receipt is accepted; crossed generation and observed byte facts fail. - `test_packet_requires_complete_canonical_guide_set`: omission, extra, swapped ingest/source, order and media; valid control before each negative commit. - `test_packet_and_ingest_facts_are_immutable`: direct UPDATE/DELETE/TRUNCATE for From f1e1077f6fb39f3542aee69a2f781268dd8d56ce Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 03:36:58 +0100 Subject: [PATCH 4/9] feat(rev): persist immutable exact review packets --- .ci/behavior-ownership/partition.v1.json | 14 +- .commitrail/INDEX.md | 14 +- .../initiatives/WS-ARCH-001/OVERVIEW.md | 8 +- .../WS-ARCH-001/planning/CHUNK_MAP.md | 2 +- .../initiatives/WS-ARCH-001/planning/PLAN.md | 4 +- .../WS-ARCH-001-04E-canonical-allow-review.md | 2 +- .../initiatives/WS-ART-001/OVERVIEW.md | 6 +- .../initiatives/WS-ART-001/WS-ART-001-07A1.md | 8 + .../initiatives/WS-AUTH-001/OVERVIEW.md | 8 +- .../WS-AUTH-001/planning/CHUNK_MAP.md | 2 +- .../initiatives/WS-AUTH-001/planning/PLAN.md | 2 +- .../initiatives/WS-AUTH-003/OVERVIEW.md | 10 +- .../initiatives/WS-CON-001/OVERVIEW.md | 8 +- .../initiatives/WS-POL-003/OVERVIEW.md | 8 +- .../WS-POL-003/planning/CHUNK_MAP.md | 2 +- .../initiatives/WS-POL-003/planning/PLAN.md | 2 +- .../initiatives/WS-REV-001/OVERVIEW.md | 13 +- .../initiatives/WS-REV-001/WS-REV-001-03B.md | 18 +- README.md | 3 +- backend/alembic/env.py | 4 +- .../alembic/versions/0012_review_packet.py | 241 ++++++++ backend/app/db/models.py | 2 + .../modules/artifacts/api/review_packet.py | 6 +- backend/app/modules/projects/models.py | 2 +- .../app/modules/reviews/packet/__init__.py | 1 + backend/app/modules/reviews/packet/models.py | 156 +++++ .../app/modules/reviews/packet/repository.py | 155 +++++ backend/app/modules/reviews/packet/schemas.py | 28 + backend/scripts/behavior_ownership.py | 2 + backend/scripts/identifier_inventory.py | 1 + backend/scripts/test_lane_catalogue.py | 4 + .../artifacts/test_review_packet_contract.py | 18 +- backend/tests/conftest.py | 7 +- backend/tests/reviews/packet/__init__.py | 0 backend/tests/reviews/packet/support.py | 184 ++++++ .../tests/reviews/packet/test_migration.py | 75 +++ .../tests/reviews/packet/test_repository.py | 261 +++++++++ backend/tests/reviews/packet/test_storage.py | 532 ++++++++++++++++++ backend/tests/test_alembic.py | 2 +- backend/tests/test_behavior_ownership.py | 11 + backend/tests/test_ci_lane_catalogue.py | 3 + backend/tests/test_identifier_schema.py | 1 + docs/architecture_data_model.md | 25 +- .../authorization_activation_custody.md | 4 +- docs/roadmap_status.md | 16 +- docs/spec_artifact_storage_service.md | 5 +- docs/spec_review_lifecycle.md | 42 +- 47 files changed, 1821 insertions(+), 101 deletions(-) create mode 100644 backend/alembic/versions/0012_review_packet.py create mode 100644 backend/app/modules/reviews/packet/__init__.py create mode 100644 backend/app/modules/reviews/packet/models.py create mode 100644 backend/app/modules/reviews/packet/repository.py create mode 100644 backend/app/modules/reviews/packet/schemas.py create mode 100644 backend/tests/reviews/packet/__init__.py create mode 100644 backend/tests/reviews/packet/support.py create mode 100644 backend/tests/reviews/packet/test_migration.py create mode 100644 backend/tests/reviews/packet/test_repository.py create mode 100644 backend/tests/reviews/packet/test_storage.py diff --git a/.ci/behavior-ownership/partition.v1.json b/.ci/behavior-ownership/partition.v1.json index fb71dd007..5c9a3529e 100644 --- a/.ci/behavior-ownership/partition.v1.json +++ b/.ci/behavior-ownership/partition.v1.json @@ -1284,6 +1284,18 @@ "group": "lifecycle", "target": "backend/app/modules/reviews/models.py" }, + { + "group": "lifecycle", + "target": "backend/app/modules/reviews/packet/models.py" + }, + { + "group": "lifecycle", + "target": "backend/app/modules/reviews/packet/repository.py" + }, + { + "group": "lifecycle", + "target": "backend/app/modules/reviews/packet/schemas.py" + }, { "group": "lifecycle", "target": "backend/app/modules/reviews/repository.py" @@ -1537,7 +1549,7 @@ "target": "backend/scripts/validate_test_lane_evidence.py" } ], - "authority_digest": "7ac9e6bbb24c63ce9cbef90541f4c0025ee8a0ebad01caf02f288ea047bff52f", + "authority_digest": "7a773f1851a863fc3fa1da772411df7e58d9e09cc0a3ee2399715b3cc4ab73b1", "protected_base_commit": "7676ce4347db0c9694962a9b587a20765e16eac6", "schema": "workstream.behavior-ownership-partition.v1" } diff --git a/.commitrail/INDEX.md b/.commitrail/INDEX.md index e2aebd63a..3811ad848 100644 --- a/.commitrail/INDEX.md +++ b/.commitrail/INDEX.md @@ -8,13 +8,13 @@ for current product capability. |---|---|---| | [WS-DB-002](initiatives/WS-DB-002/OVERVIEW.md) | Complete | Shared UUIDv7 record generation, native-UUID relationships and fresh v0.1 baseline; natural-owner retry custody and aligned CI/local setup | | [WS-MCP-002](initiatives/WS-MCP-002/OVERVIEW.md) | Planned | Three self-service tools delivered through WS-MCP-002-02; 24 tools remain and WS-MCP-002-03 administrative reads are next | -| [WS-ARCH-001](initiatives/WS-ARCH-001/OVERVIEW.md) | Planned | ARCH-04E1A immutable route-neutral TASK source storage, detached facts and type-only accepted-effects contract are delivered; next build REV-03B packet and REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence foundations before 04E1B/04E2/04E3 routing and 04F remediation | -| [WS-ART-001](initiatives/WS-ART-001/OVERVIEW.md) | Planned | ART-07A1 metadata-only packet contract and ARCH-04E1A source facts are delivered; REV-03B packet storage, REV-04A Review storage and shared acceptance precede routing, 04F remediation and public intake | -| [WS-AUTH-001](initiatives/WS-AUTH-001/OVERVIEW.md) | Planned | ARCH-04E1A source facts are delivered without routing authority; REV-03B packet and REV-04A Review storage, shared acceptance foundations and hidden 04E1B proof precede exact 04E2 activation and 04E3 live composition | -| [WS-CON-001](initiatives/WS-CON-001/OVERVIEW.md) | Planned | ARCH-04E1A source facts are delivered; REV-03B packet and REV-04A Review storage, then REV-04B FinalAcceptance persistence, CON-03C/07 and the shared REV-12A/CON fence foundation are next before either acceptance trigger composes shared effects | -| [WS-AUTH-003](initiatives/WS-AUTH-003/OVERVIEW.md) | Planned | ARCH-04E1A source facts and source-neutral types are delivered without runtime composition; continue with REV-03B packet and REV-04A Review storage, then shared acceptance foundations and later 04E1B/04E2/04E3 route | -| [WS-POL-003](initiatives/WS-POL-003/OVERVIEW.md) | Planned | ARCH-04E1A source facts and ART-07A1 packet types are delivered; REV-03B packet and REV-04A Review storage come next, while false activation remains unavailable until shared acceptance, exact routing authority, live composition and 04F remediation are proven | -| [WS-REV-001](initiatives/WS-REV-001/OVERVIEW.md) | Planned | ARCH-04E1A TASK source foundation delivered; ART-07A1 packet types delivered; REV-03B packet and REV-04A Review storage, then REV-04B FinalAcceptance, CON-03C/07 and existing REV-12A/CON fence foundations are next; human hidden review work remains independently dependency-gated | +| [WS-ARCH-001](initiatives/WS-ARCH-001/OVERVIEW.md) | Planned | ARCH-04E1A immutable route-neutral TASK source storage, detached facts and type-only accepted-effects contract are delivered; next build complete REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence foundations before 04E1B/04E2/04E3 routing and 04F remediation | +| [WS-ART-001](initiatives/WS-ART-001/OVERVIEW.md) | Planned | ART-07A1 packet types, REV-03B packet storage and ARCH-04E1A source facts are delivered; complete REV-04A Review storage and shared acceptance precede routing, 04F remediation and public intake | +| [WS-AUTH-001](initiatives/WS-AUTH-001/OVERVIEW.md) | Planned | ARCH-04E1A source facts are delivered without routing authority; complete REV-04A Review storage, shared acceptance foundations and hidden 04E1B proof precede exact 04E2 activation and 04E3 live composition | +| [WS-CON-001](initiatives/WS-CON-001/OVERVIEW.md) | Planned | ARCH-04E1A source facts are delivered; complete REV-04A Review storage, then REV-04B FinalAcceptance persistence, CON-03C/07 and the shared REV-12A/CON fence foundation are next before either acceptance trigger composes shared effects | +| [WS-AUTH-003](initiatives/WS-AUTH-003/OVERVIEW.md) | Planned | ARCH-04E1A source facts and source-neutral types are delivered without runtime composition; continue with complete REV-04A Review storage, then shared acceptance foundations and later 04E1B/04E2/04E3 route | +| [WS-POL-003](initiatives/WS-POL-003/OVERVIEW.md) | Planned | ARCH-04E1A source facts, ART-07A1 packet types and REV-03B packet storage are delivered; complete REV-04A Review storage comes next, while false activation remains unavailable until shared acceptance, exact routing authority, live composition and 04F remediation are proven | +| [WS-REV-001](initiatives/WS-REV-001/OVERVIEW.md) | Planned | ARCH-04E1A TASK source foundation delivered; ART-07A1 packet types and REV-03B packet storage delivered; complete REV-04A Review storage, then REV-04B FinalAcceptance, CON-03C/07 and existing REV-12A/CON fence foundations are next; human hidden review work remains independently dependency-gated | | [WS-QUAL-002](initiatives/WS-QUAL-002/OVERVIEW.md) | Planned | Populate subsystem ownership before changed-line mutation work | | [WS-QUAL-003](initiatives/WS-QUAL-003/OVERVIEW.md) | Planned | Audit and prune test proof, add missing safety cases, decompose oversized test modules | | [WS-XINT-002](initiatives/WS-XINT-002/OVERVIEW.md) | Planned | Remaining ART/AUTH activation edges only | diff --git a/.commitrail/initiatives/WS-ARCH-001/OVERVIEW.md b/.commitrail/initiatives/WS-ARCH-001/OVERVIEW.md index d02a15af9..e935cf4f7 100644 --- a/.commitrail/initiatives/WS-ARCH-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-ARCH-001/OVERVIEW.md @@ -9,8 +9,8 @@ Exact pre-cutover work record: [`STATUS.md`](pre-cutover/STATUS.md), - Disposition: Planned - Delivered prerequisite: [ART-07A1](../WS-ART-001/WS-ART-001-07A1.md) supplies - metadata-only packet types; REV-03B packet storage and REV-04A Review storage - precede shared FinalAcceptance. No packet resolver or human runtime is live. + metadata-only packet types. REV-03B packet storage is delivered; complete + REV-04A Review storage is next before shared FinalAcceptance. No packet resolver or human runtime is live. - Completed boundary: through 02H, [CP05](WS-ARCH-001-CP05.md), [CP06](WS-ARCH-001-CP06.md), [CP07](WS-ARCH-001-CP07.md), [ARCH-03A](WS-ARCH-001-03A.md), and [ARCH-04A consolidation](WS-ARCH-001-04A.md) canonical post-submit contracts/conformance. @@ -26,7 +26,9 @@ Exact pre-cutover work record: [`STATUS.md`](pre-cutover/STATUS.md), authority. The source table has no writer, reader, handler, current pointer, routing authority or acceptance effect implementation. False is proven only as a scalar DTO value because activation still rejects it. -- Next usable boundary: REV-03B normalized packet persistence, then complete +- Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact + lease packets with normalized live guide ingests; no resolver or byte authority. +- Next usable boundary: complete REV-04A Review storage and shared REV-04B FinalAcceptance persistence, CON-03C/07 and the existing REV-12A/CON fence foundation before shared acceptance composition, then ARCH-04E1B/04E2/04E3 and ARCH-04F. Output-file diff --git a/.commitrail/initiatives/WS-ARCH-001/planning/CHUNK_MAP.md b/.commitrail/initiatives/WS-ARCH-001/planning/CHUNK_MAP.md index 6b6e85444..971ddd5cd 100644 --- a/.commitrail/initiatives/WS-ARCH-001/planning/CHUNK_MAP.md +++ b/.commitrail/initiatives/WS-ARCH-001/planning/CHUNK_MAP.md @@ -41,7 +41,7 @@ public intake remains deferred to ARCH-02I. | [WS-ARCH-001-04D1](../WS-ARCH-001-04D1.md) | Canonical terminal ART material custody | L1 | Complete; valid retained history preserved; invalid upgrades refused | | [WS-ARCH-001-04D2](../WS-ARCH-001-04D2.md) | AUTH exact fixed-service post-submit activation (replaces XINT-06B) | L1 | Complete: exact input, execute and finalize authority; output write/bind remains unavailable | | [WS-ARCH-001-04E1A](../WS-ARCH-001-04E1A.md) | Route-neutral immutable source schema and shared accepted-effects types | L1 | Complete; no runtime writer/reader, routing authority, current pointer or effects implementation; false proof is scalar transport only | -| [WS-ARCH-001-04E](chunks/WS-ARCH-001-04E-canonical-allow-review.md) | TASK current routing: true to canonical `allow_review`, false/pass to shared acceptance | L1 | Delivered source 04E1A -> REV-03B packet and REV-04A Review storage, then shared REV-04B/CON-03C/07 plus REV-12A/CON fence foundation -> hidden 04E1B -> AUTH 04E2 -> live 04E3, plus 04D2/OUTBOX-02; false activation also requires 04F remediation | +| [WS-ARCH-001-04E](chunks/WS-ARCH-001-04E-canonical-allow-review.md) | TASK current routing: true to canonical `allow_review`, false/pass to shared acceptance | L1 | Delivered source 04E1A -> complete REV-04A Review storage, then shared REV-04B/CON-03C/07 plus REV-12A/CON fence foundation -> hidden 04E1B -> AUTH 04E2 -> live 04E3, plus 04D2/OUTBOX-02; false activation also requires 04F remediation | | [WS-ARCH-001-03D](../WS-ARCH-001-03D.md) | Exact activated historical guide through hidden durable intake; obsolete lookup removed | L1 | Complete; hidden exact post-submit materialization, ARCH-04B2 output custody, ARCH-04C execution, ARCH-04D1/04D2 custody/authority and ARCH-04E1A source-only facts/types delivered; public cutover remains deferred | | [WS-ARCH-001-04F](chunks/WS-ARCH-001-04F-checker-remediation.md) | Contributor-correctable checker failures and same-lineage admission-backed replacement Submission | L1 | Planned after 04E; replaces XINT-05C, required before public 02I, not before REV begins from `allow_review` | diff --git a/.commitrail/initiatives/WS-ARCH-001/planning/PLAN.md b/.commitrail/initiatives/WS-ARCH-001/planning/PLAN.md index cf0da3705..1aff7d2d8 100644 --- a/.commitrail/initiatives/WS-ARCH-001/planning/PLAN.md +++ b/.commitrail/initiatives/WS-ARCH-001/planning/PLAN.md @@ -57,7 +57,7 @@ checker-remediation boundary before public Submission cutover. | CON-02B | AUTH-OUTBOX-01 | Complete: shared hidden dispatcher/claim fencing, typed handlers and recovery | | AUTH-OUTBOX-02 | CON-02B exact hidden manifest | Exact dispatcher mechanics only; no feature authority | | [ARCH-04E1A](../WS-ARCH-001-04E1A.md) | ARCH-04C/04D2 | Complete: route-neutral immutable TASK source schema/detached facts and source-neutral accepted-effects types; no runtime writer/reader, handlers, current pointer, routing authority, acceptance implementation or REV dependency | -| ARCH-04E1B | ARCH-04E1A, CON-02B hidden contract; ART-07A1 types -> REV-03B/04A storage -> shared REV-04B + CON-03C/07 + REV-12A shared fence foundation for false | TASK hidden handlers; consume one shared acceptance operation on false/pass | +| ARCH-04E1B | ARCH-04E1A, CON-02B hidden contract; ART-07A1 types -> delivered REV-03B packet -> complete REV-04A storage -> shared REV-04B + CON-03C/07 + REV-12A shared fence foundation for false | TASK hidden handlers; consume one shared acceptance operation on false/pass | | Scoped XINT-003-08B controller activation | Early existing REV-12A foundation and hidden shared acceptance/writer/observation proof | Existing Operator lifecycle-control action for the bounded shared manifest, not human runtime | | ARCH-04E2 | ARCH-04E1B; scoped XINT-003-08B controller activation for false | AUTH exact TASK routing authority | | ARCH-04E3 | ARCH-04E2, ARCH-04D2, AUTH-OUTBOX-02; shared acceptance proof for false | TASK live dispatch/routing composition: true to allow_review, false/pass to shared acceptance when proven | @@ -98,7 +98,7 @@ new permission requirement. Delivered supporting foundations are [ARCH-04B2](../WS-ARCH-001-04B2.md), [AUTH-OUTBOX-01/02](../../WS-AUTH-001/planning/PLAN.md#ws-auth-001-outbox-01--unavailable-dispatcher-contract), and [CON-02B](../../WS-CON-001/OVERVIEW.md#con-02b-current-dispatcher-contract). -The remaining routing sequence starts with REV-03B packet and REV-04A Review storage, then shared REV-04B/CON-03C/07 and the +The remaining routing sequence starts with complete REV-04A Review storage, then shared REV-04B/CON-03C/07 and the existing REV-12A/CON fence foundation before shared acceptance composition, then [ARCH-04E1B/04E2/04E3](chunks/WS-ARCH-001-04E-canonical-allow-review.md#current-bounded-sequence) and ARCH-04F. ARCH-04E1A is delivered as the source-only predecessor. diff --git a/.commitrail/initiatives/WS-ARCH-001/planning/chunks/WS-ARCH-001-04E-canonical-allow-review.md b/.commitrail/initiatives/WS-ARCH-001/planning/chunks/WS-ARCH-001-04E-canonical-allow-review.md index bf77a15ba..b04cd7ec1 100644 --- a/.commitrail/initiatives/WS-ARCH-001/planning/chunks/WS-ARCH-001-04E-canonical-allow-review.md +++ b/.commitrail/initiatives/WS-ARCH-001/planning/chunks/WS-ARCH-001-04E-canonical-allow-review.md @@ -28,7 +28,7 @@ current TASK children do not implement REV/CON internals: consume the are delivered after 04C, without REV dependency, runtime participant or handlers. REV-04B can reference that schema only after its other source prerequisites: [ART-07A1 packet types](../../../WS-ART-001/WS-ART-001-07A1.md) are delivered; -REV-03B normalized packet storage and complete REV-04A Review storage follow. +REV-03B normalized packet storage is delivered; complete REV-04A Review storage is next. Shared REV-04B/CON-03C/07 and the early existing REV-12A/CON fence foundation are hard dependencies of false handler composition, not of this diff --git a/.commitrail/initiatives/WS-ART-001/OVERVIEW.md b/.commitrail/initiatives/WS-ART-001/OVERVIEW.md index 29b8a9408..5f4cb4bcd 100644 --- a/.commitrail/initiatives/WS-ART-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-ART-001/OVERVIEW.md @@ -19,7 +19,9 @@ and the [capability ledger](../../../docs/roadmap_status.md). zero-output catalogue. - Delivered contract: [ART-07A1](WS-ART-001-07A1.md) defines exact, metadata-only reviewer packet membership. It supplies no resolver or byte authority. -- Next usable boundary: REV-03B normalized packet persistence, then REV-04A +- Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact + lease packets with normalized live guide ingests; no resolver or byte authority. +- Next usable boundary: complete REV-04A Review storage before shared acceptance. ARCH-04F remediation and public intake follow acceptance and routing composition. - Governing sources: artifact specifications, `ArtifactStore`, @@ -48,7 +50,7 @@ must prove exact approved lineage at preparation, consumption and binding. custody, ARCH-04C hidden durable execution, ARCH-04D2 fixed-service authority and ARCH-04E1A source-only material lineage are delivered. 2. ART-07A1 metadata-only membership types are delivered. REV-03B packet - storage and complete REV-04A Review storage precede shared FinalAcceptance, + storage is delivered. Complete REV-04A Review storage precedes shared FinalAcceptance, CON participation and the shared fence, then acceptance/routing composition. 3. ARCH-04F owns checker-remediation resubmission using existing ART ports; later reviewer-requested revision remains a separate REV boundary. Add those dependencies before public Submission diff --git a/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md b/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md index d1316bced..cf094ccfa 100644 --- a/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md +++ b/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md @@ -193,3 +193,11 @@ Implementation review tightened native Python UUID validation (JSON decoding remains supported) and reconciled later human-runtime steps with storage already required by automated acceptance. The substitution regression rejects valid UUID strings at request and both member boundaries; no compatibility coercion remains. + +## Current owner correction + +[REV-03B](../WS-REV-001/WS-REV-001-03B.md) replaces the originally delivered +`guide_binding_id` with `ingest_id` from live guide uploads. The former target is +retained extraction evidence whose writes are sealed. No alias, revived writer +or data deletion is introduced. The original delivery record above is historical; +current packet consumers use only the corrected contract. diff --git a/.commitrail/initiatives/WS-AUTH-001/OVERVIEW.md b/.commitrail/initiatives/WS-AUTH-001/OVERVIEW.md index e4e98d6c7..dcf0d4380 100644 --- a/.commitrail/initiatives/WS-AUTH-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-AUTH-001/OVERVIEW.md @@ -11,8 +11,8 @@ Historical pre-cutover work records: [`STATUS.md`](pre-cutover/STATUS.md), - Disposition: Planned - Delivered prerequisite: [ART-07A1](../WS-ART-001/WS-ART-001-07A1.md) supplies - metadata-only packet types; REV-03B packet storage and REV-04A Review storage - precede shared FinalAcceptance. No packet resolver or human runtime is live. + metadata-only packet types. REV-03B packet storage is delivered; complete + REV-04A Review storage is next before shared FinalAcceptance. No packet resolver or human runtime is live. - Intent: provide deny-default, project-scoped authority with canonical human and service identities and attributable audit evidence. @@ -49,7 +49,9 @@ Historical pre-cutover work records: [`STATUS.md`](pre-cutover/STATUS.md), and minimal writers and ARCH-03A internal guide context. POL-07B internal phase composition is delivered. The dispatcher registers only exact assignment invalidation. Future checker routing still requires its separate exact authority and handler. -- Next usable boundary: REV-03B packet and REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence +- Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact + lease packets with normalized live guide ingests; no resolver or byte authority. +- Next usable boundary: complete REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence foundations precede shared acceptance composition and hidden ARCH-04E1B; ARCH-04E2 then owns exact routing activation before 04E3 live composition. - Governing source: `docs/spec_authorization_service.md`, authorization code, diff --git a/.commitrail/initiatives/WS-AUTH-001/planning/CHUNK_MAP.md b/.commitrail/initiatives/WS-AUTH-001/planning/CHUNK_MAP.md index 18882bd86..d25bdb4b3 100644 --- a/.commitrail/initiatives/WS-AUTH-001/planning/CHUNK_MAP.md +++ b/.commitrail/initiatives/WS-AUTH-001/planning/CHUNK_MAP.md @@ -1,7 +1,7 @@ # WS-AUTH-001 — Current pre-review activation map The delivered [ART-07A1 metadata contract](../../WS-ART-001/WS-ART-001-07A1.md) -now precedes REV-03B packet persistence and complete REV-04A Review storage, +and REV-03B normalized packet persistence are delivered. Complete REV-04A Review storage is next, then shared REV-04B/CON acceptance foundations. These are storage prerequisites; no live human-review queue or endpoint is required for automated acceptance. diff --git a/.commitrail/initiatives/WS-AUTH-001/planning/PLAN.md b/.commitrail/initiatives/WS-AUTH-001/planning/PLAN.md index 350c57883..3f9e74252 100644 --- a/.commitrail/initiatives/WS-AUTH-001/planning/PLAN.md +++ b/.commitrail/initiatives/WS-AUTH-001/planning/PLAN.md @@ -1,7 +1,7 @@ # WS-AUTH-001 — Current pre-review activation plan The delivered [ART-07A1 metadata contract](../../WS-ART-001/WS-ART-001-07A1.md) -now precedes REV-03B packet persistence and complete REV-04A Review storage, +and REV-03B normalized packet persistence are delivered. Complete REV-04A Review storage is next, then shared REV-04B/CON acceptance foundations. These are storage prerequisites; no live human-review queue or endpoint is required for automated acceptance. diff --git a/.commitrail/initiatives/WS-AUTH-003/OVERVIEW.md b/.commitrail/initiatives/WS-AUTH-003/OVERVIEW.md index 93413aa8a..64eaccfec 100644 --- a/.commitrail/initiatives/WS-AUTH-003/OVERVIEW.md +++ b/.commitrail/initiatives/WS-AUTH-003/OVERVIEW.md @@ -6,8 +6,8 @@ Exact pre-cutover work record: [`STATUS.md`](pre-cutover/STATUS.md), - Disposition: Planned - Delivered prerequisite: [ART-07A1](../WS-ART-001/WS-ART-001-07A1.md) supplies - metadata-only packet types; REV-03B packet storage and REV-04A Review storage - precede shared FinalAcceptance. No packet resolver or human runtime is live. + metadata-only packet types. REV-03B packet storage is delivered; complete + REV-04A Review storage is next before shared FinalAcceptance. No packet resolver or human runtime is live. - Completed boundary: recovery foundation and [TASK/checker authorization cleanup](WS-AUTH-003-TASKCHECKER.md). - Intent: route public authorization capability through `authorization.api` @@ -18,8 +18,10 @@ Exact pre-cutover work record: [`STATUS.md`](pre-cutover/STATUS.md), [ARCH-04B2 output custody](../WS-ARCH-001/WS-ARCH-001-04B2.md), ARCH-03D hidden intake and [AUTH-18 public manager activation](../WS-AUTH-001/WS-AUTH-001-18.md). They install no routing action, handler or runtime composition. -- Next usable boundary: continue canonical boundary recovery through REV-03B - packet and complete REV-04A Review storage, then shared REV/CON/fence +- Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact + lease packets with normalized live guide ingests; no resolver or byte authority. +- Next usable boundary: continue canonical boundary recovery through complete + REV-04A Review storage, then shared REV/CON/fence foundations and later ARCH-04E1B/04E2/04E3 routing. Submission/checker history uses canonical authority; the alternate gate lifecycle is removed. Continue shrinking the canonical import ledger as implementation diff --git a/.commitrail/initiatives/WS-CON-001/OVERVIEW.md b/.commitrail/initiatives/WS-CON-001/OVERVIEW.md index 656a200f7..77f587b65 100644 --- a/.commitrail/initiatives/WS-CON-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-CON-001/OVERVIEW.md @@ -5,8 +5,8 @@ and the [capability ledger](../../../docs/roadmap_status.md). - Disposition: Planned - Delivered prerequisite: [ART-07A1](../WS-ART-001/WS-ART-001-07A1.md) supplies - metadata-only packet types; REV-03B packet storage and REV-04A Review storage - precede shared FinalAcceptance. No packet resolver or human runtime is live. + metadata-only packet types. REV-03B packet storage is delivered; complete + REV-04A Review storage is next before shared FinalAcceptance. No packet resolver or human runtime is live. - Completed boundary: public Finance ContributionPolicy administration, exact Finance Authority, CP06 selected-policy validation and CP07 internal guide @@ -36,7 +36,9 @@ and the [capability ledger](../../../docs/roadmap_status.md). and minimal writers, ARCH-03A internal guide context, [CP07 activation/binding](../WS-ARCH-001/WS-ARCH-001-CP07.md) and [AUTH-12H live authority](../WS-AUTH-001/WS-AUTH-001-12H.md), before task readiness. -- Next usable boundary: REV-03B packet and REV-04A Review storage, then +- Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact + lease packets with normalized live guide ingests; no resolver or byte authority. +- Next usable boundary: complete REV-04A Review storage, then REV-04B FinalAcceptance persistence and CON-03C/07 and the existing shared REV-12A/CON fence foundation before one shared acceptance operation serves both the human and automatic triggers. diff --git a/.commitrail/initiatives/WS-POL-003/OVERVIEW.md b/.commitrail/initiatives/WS-POL-003/OVERVIEW.md index 8b457c68a..9be04d753 100644 --- a/.commitrail/initiatives/WS-POL-003/OVERVIEW.md +++ b/.commitrail/initiatives/WS-POL-003/OVERVIEW.md @@ -17,8 +17,8 @@ Exact pre-cutover work record: [`STATUS.md`](pre-cutover/STATUS.md), - Disposition: Planned - Delivered prerequisite: [ART-07A1](../WS-ART-001/WS-ART-001-07A1.md) supplies - metadata-only packet types; REV-03B packet storage and REV-04A Review storage - precede shared FinalAcceptance. No packet resolver or human runtime is live. + metadata-only packet types. REV-03B packet storage is delivered; complete + REV-04A Review storage is next before shared FinalAcceptance. No packet resolver or human runtime is live. - Completed boundary: automatic unified execution, deterministic projections, immutable setup finalization, current-authority replay and one public guide @@ -61,7 +61,9 @@ Exact pre-cutover work record: [`STATUS.md`](pre-cutover/STATUS.md), Earlier development schemas require no backward-compatibility paths. The existing ReviewPolicy boolean is delivered; false has scalar DTO proof only and automated acceptance remains unavailable. -- Next usable boundary: REV-03B packet and REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence +- Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact + lease packets with normalized live guide ingests; no resolver or byte authority. +- Next usable boundary: complete REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence foundations, then shared acceptance composition and ARCH-04E1B/04E2/04E3. ARCH-04F remediation still precedes enabling false. - Governing sources: project-guide specifications, authorization and diff --git a/.commitrail/initiatives/WS-POL-003/planning/CHUNK_MAP.md b/.commitrail/initiatives/WS-POL-003/planning/CHUNK_MAP.md index 15bb7cb6a..6342fb9d6 100644 --- a/.commitrail/initiatives/WS-POL-003/planning/CHUNK_MAP.md +++ b/.commitrail/initiatives/WS-POL-003/planning/CHUNK_MAP.md @@ -1,7 +1,7 @@ # Chunk Map: WS-POL-003 - Unified Project Guide Compilation The delivered [ART-07A1 metadata contract](../../WS-ART-001/WS-ART-001-07A1.md) -now precedes REV-03B packet persistence and complete REV-04A Review storage, +and REV-03B normalized packet persistence are delivered. Complete REV-04A Review storage is next, then shared REV-04B/CON acceptance foundations. These are storage prerequisites; no live human-review queue or endpoint is required for automated acceptance. diff --git a/.commitrail/initiatives/WS-POL-003/planning/PLAN.md b/.commitrail/initiatives/WS-POL-003/planning/PLAN.md index ff6a70309..ac90f7f9c 100644 --- a/.commitrail/initiatives/WS-POL-003/planning/PLAN.md +++ b/.commitrail/initiatives/WS-POL-003/planning/PLAN.md @@ -1,7 +1,7 @@ # Plan: WS-POL-003 - Unified Project Guide Compilation The delivered [ART-07A1 metadata contract](../../WS-ART-001/WS-ART-001-07A1.md) -now precedes REV-03B packet persistence and complete REV-04A Review storage, +and REV-03B normalized packet persistence are delivered. Complete REV-04A Review storage is next, then shared REV-04B/CON acceptance foundations. These are storage prerequisites; no live human-review queue or endpoint is required for automated acceptance. diff --git a/.commitrail/initiatives/WS-REV-001/OVERVIEW.md b/.commitrail/initiatives/WS-REV-001/OVERVIEW.md index 41bcde450..058544692 100644 --- a/.commitrail/initiatives/WS-REV-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-REV-001/OVERVIEW.md @@ -6,17 +6,20 @@ of review/revision behavior. The downstream owner contracts remain separate. - Disposition: Planned - Delivered prerequisite: [ART-07A1](../WS-ART-001/WS-ART-001-07A1.md) supplies - metadata-only packet types; REV-03B packet storage and REV-04A Review storage - precede shared FinalAcceptance. No packet resolver or human runtime is live. + metadata-only packet types. REV-03B packet storage is delivered; complete + REV-04A Review storage is next before shared FinalAcceptance. No packet resolver or human runtime is live. -- Completed boundary: queue admission and ReviewLease persistence through 03A2. +- Completed boundary: queue admission and ReviewLease persistence through 03A2, + plus normalized immutable packet storage through 03B. - Intent: ensure the authorized reviewer evaluates the exact verified artifact under the locked policy version and produces attributable outcomes. - Delivered upstream boundary: ARCH-04E1A provides immutable route-neutral TASK source storage, detached source facts and the type-only accepted-effects Protocol. It provides no writer, reader, handler, routing authority, current pointer or acceptance implementation. -- Next usable boundary: REV-03B normalized packet persistence, then complete +- Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact + lease packets with normalized live guide ingests; no resolver or byte authority. +- Next usable boundary: complete REV-04A Review storage and REV-04B shared FinalAcceptance persistence, CON-03C/07 and the existing REV-12A/CON fence foundation under the canonical order; human hidden behavior may continue independently behind exact AUTH, @@ -62,7 +65,7 @@ live human queues, ReviewLeases or decision endpoints. Human lifecycle work remains required for v0.1, but need not delay the first automated end-to-end proof. No adjudication setting or behavior is included. -1. `03B`: normalized reviewer packet persistence consumes the delivered +1. `03B` is complete: normalized reviewer packet persistence consumes the delivered [ART-07A1 exact membership contract](../WS-ART-001/WS-ART-001-07A1.md). Next complete REV-04A Review-source storage; neither step activates human review. 2. Continue hidden claim/revision behavior against canonical `allow_review`, diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index 25ab4b5b6..904130f72 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -1,7 +1,7 @@ # REV-03B — Immutable normalized reviewer packet persistence - Initiative: `WS-REV-001` -- Durable disposition: `Planned` +- Durable disposition: `Complete` - Risk: L1 (bounded schema, immutable evidence and owner contract correction). - Intended merge outcome: REV persists one exact metadata packet per ReviewLease with normalized guide members; complete REV-04A Review storage is next. No claim, resolver, byte access or acceptance is activated. @@ -12,7 +12,7 @@ sequence, so automated acceptance can become the first complete runtime path without fabricating a Review. Build on merged ART-07A1 and the existing queue, lease, Submission, checker and activated guide owners. -Current main `7754703f` has no packet persistence. Discovery also found that +At discovery, main `7754703f` had no packet persistence. Discovery also found that ART-07A1's `guide_binding_id` targets the retired extraction binding table. `retired_guide_material_write_guard` rejects every write to that table. The live upload and guide manifest use `GuideSourceArtifactIngest.id`, exposed as @@ -42,6 +42,7 @@ read-only. The complete packet remains metadata-only. - `backend/tests/reviews/packet/test_migration.py` - `backend/tests/conftest.py`: exact new resettable/guarded tables and measured schema fingerprint; no weaker validation. - `backend/tests/test_alembic.py`: exact revision graph. +- `backend/scripts/identifier_inventory.py` and `backend/tests/test_identifier_schema.py`: classify only the new natural packet/source-item primary key; preserve all UUID guards. - `backend/scripts/test_lane_catalogue.py` and `backend/tests/test_ci_lane_catalogue.py`: additive registration in existing task lanes. - `backend/scripts/behavior_ownership.py`, `backend/tests/test_behavior_ownership.py`, `.ci/behavior-ownership/partition.v1.json`: exact three-module REV addition and neighbor-rejection proof. @@ -96,8 +97,7 @@ binding table. The existing header request retains all eleven scope fields. Exactly one required ZIP is represented by non-null header fields; no separate one-to-one item table. All IDs/references use native PostgreSQL UUID, with native -Python UUID at new typed boundaries. Existing owner string representations stay -at existing boundaries. `result_id` is the aggregate CheckerRun.result_id. +Python UUID at new typed boundaries. ORM references preserve the referenced owner's string-versus-UUID representation; new detached packet boundaries use UUID. `result_id` is the aggregate CheckerRun.result_id. `review_packet_guide_items` columns: `packet_id`, `source_item_id` (composite primary key), `ingest_id`, `item_order`, `logical_role`, `media_type`. @@ -111,7 +111,7 @@ ART membership JSON: request, submission and ordered guide members, with native UUIDs rendered as strings. It excludes packet UUID, lease identity, generation and creation time, so future claim can compute it before allocating a lease. Those independent identities remain explicitly bound by AUTH. SQL recomputes -the same digest using the existing canonical JSON function and pgcrypto digest; +the same digest using the existing canonical JSON function and PostgreSQL SHA-256; caller-supplied false digests fail at commit. The positive Python/SQL parity proof includes all fields and a changed ordered member. No second serializer. @@ -235,9 +235,9 @@ and lane inventory checks; run Ruff, module boundaries, Commitrail, links and stale wording. Full hosted nine-lane and real API checks remain required, with no skipped/deselected nodes. No spreadsheet exports are currently present. -### Named future proof inventory +### Named proof inventory -These are implementation obligations, not claims of executed tests: +These tests bind the storage boundary; execution results and review freshness belong in the PR: - `test_packet_matches_canonical_owners_and_digest`: every header/nested owner value and Python/SQL digest parity, including changed ordered membership. @@ -248,7 +248,9 @@ These are implementation obligations, not claims of executed tests: - `test_packet_requires_active_exact_lease`: terminal and sibling lease insertions. - `test_packet_requires_committed_guide_upload`: prepared-only ingest cannot pass; independently crossed content, replica, namespace, receipt, ingest byte and - media identities fail after a valid operation-receipt control. + media identities fail after a valid operation-receipt control. Namespace + substitution is rejected by ART's existing singleton namespace FK before packet + validation; no claim that this impossible source reaches the later packet guard. - `test_packet_accepts_observed_confirmed_upload`: genuine observed-confirmed recovery receipt is accepted; crossed generation and observed byte facts fail. - `test_packet_requires_complete_canonical_guide_set`: omission, extra, swapped diff --git a/README.md b/README.md index 426843e7b..39bd97019 100644 --- a/README.md +++ b/README.md @@ -173,7 +173,8 @@ Findings and policy proposals retain document-access evidence. [ART-07A1](.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) provides strict metadata-only reviewer packet types, not a resolver or byte-access capability. -Next are REV-03B normalized packet storage and complete REV-04A Review storage, +REV-03B persists immutable normalized packets using live guide ingest identities. +Complete REV-04A Review storage is next, then shared FinalAcceptance. These internal prerequisites do not require live human review before the first automated acceptance path. diff --git a/backend/alembic/env.py b/backend/alembic/env.py index 1892affdb..0e61a9890 100644 --- a/backend/alembic/env.py +++ b/backend/alembic/env.py @@ -21,7 +21,7 @@ target_metadata = Base.metadata _BASELINE_REVISION = "0001_uuid7_v01" -_CURRENT_HEAD_REVISION = "0011_task_routing_source" +_CURRENT_HEAD_REVISION = "0012_review_packet" _RECREATE_GUIDANCE = ( "Workstream v0.1 requires a fresh database; recreate this database before " "running the 0001_uuid7_v01 migration" @@ -52,7 +52,7 @@ def do_run_migrations(connection: Connection) -> None: .scalars() .all() ) - if revisions not in ((), (_BASELINE_REVISION,), ("0002_task_queue_authority",), ("0003_task_read_authority",), ("0004_task_context_authority",), ("0005_task_evidence_authority",), ("0006_history_read_authority",), ("0007_checker_output_custody",), ("0008_checker_execution",), ("0009_checker_material_lineage",), ("0010_post_submit_authority",), (_CURRENT_HEAD_REVISION,)): + if revisions not in ((), (_BASELINE_REVISION,), ("0002_task_queue_authority",), ("0003_task_read_authority",), ("0004_task_context_authority",), ("0005_task_evidence_authority",), ("0006_history_read_authority",), ("0007_checker_output_custody",), ("0008_checker_execution",), ("0009_checker_material_lineage",), ("0010_post_submit_authority",), ("0011_task_routing_source",), (_CURRENT_HEAD_REVISION,)): raise RuntimeError(_RECREATE_GUIDANCE) # The read-only preflight autobegins a SQLAlchemy transaction. End that # transaction before Alembic establishes the migration transaction; diff --git a/backend/alembic/versions/0012_review_packet.py b/backend/alembic/versions/0012_review_packet.py new file mode 100644 index 000000000..4402af095 --- /dev/null +++ b/backend/alembic/versions/0012_review_packet.py @@ -0,0 +1,241 @@ +"""Persist immutable normalized reviewer packets without activating review access.""" + +from alembic import op + +revision = "0012_review_packet" +down_revision = "0011_task_routing_source" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute("SET LOCAL search_path = pg_catalog, public, pg_temp") + op.execute(""" +CREATE TABLE public.review_packet_manifests ( + id UUID NOT NULL, + created_at TIMESTAMP WITH TIME ZONE DEFAULT clock_timestamp() NOT NULL, + review_lease_id UUID NOT NULL, + review_queue_entry_id UUID NOT NULL, + packet_manifest_generation INTEGER NOT NULL, + packet_manifest_digest VARCHAR(71) NOT NULL, + project_id UUID NOT NULL, + task_id UUID NOT NULL, + submission_id UUID NOT NULL, + submission_version INTEGER NOT NULL, + checker_run_id UUID NOT NULL, + result_id UUID NOT NULL, + guide_id UUID NOT NULL, + guide_version VARCHAR(50) NOT NULL, + source_snapshot_id UUID NOT NULL, + project_setup_run_id UUID NOT NULL, + setup_generation INTEGER NOT NULL, + submission_binding_id UUID NOT NULL, + submission_logical_role VARCHAR(32) NOT NULL, + submission_media_type VARCHAR(100) NOT NULL, + CONSTRAINT pk_review_packet_manifests PRIMARY KEY (id), + CONSTRAINT ck_review_packet_manifests_id_uuid7 CHECK ((get_byte(uuid_send(id), 6) >> 4) = 7 and (get_byte(uuid_send(id), 8) & 192) = 128), + CONSTRAINT uq_review_packet_lease UNIQUE (review_lease_id), + CONSTRAINT fk_review_packet_task FOREIGN KEY(task_id, project_id) REFERENCES public.workstream_tasks (id, project_id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_submission FOREIGN KEY(submission_id, task_id, submission_version) REFERENCES public.submissions (id, task_id, version) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_checker FOREIGN KEY(checker_run_id, task_id, submission_id) REFERENCES public.checker_runs (id, task_id, submission_id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_snapshot FOREIGN KEY(source_snapshot_id, project_id, guide_id) REFERENCES public.guide_source_snapshots (id, project_id, guide_id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_setup FOREIGN KEY(project_setup_run_id, project_id, guide_id, source_snapshot_id, setup_generation) REFERENCES public.project_setup_runs (id, project_id, guide_id, source_snapshot_id, setup_generation) ON DELETE RESTRICT, + CONSTRAINT ck_review_packet_manifests_positive_generations CHECK (submission_version > 0 and setup_generation > 0 and packet_manifest_generation > 0), + CONSTRAINT ck_review_packet_manifests_guide_version_nonblank CHECK (length(btrim(guide_version)) > 0), + CONSTRAINT ck_review_packet_manifests_digest_shape CHECK (packet_manifest_digest ~ '^sha256:[0-9a-f]{64}$'), + CONSTRAINT ck_review_packet_manifests_original_zip CHECK (submission_logical_role='submission_bundle_original' and submission_media_type='application/zip'), + CONSTRAINT fk_review_packet_manifests_review_lease_id_review_leases FOREIGN KEY(review_lease_id) REFERENCES public.review_leases (id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_manifests_review_queue_entry_id_review_cc56 FOREIGN KEY(review_queue_entry_id) REFERENCES public.review_queue_entries (id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_manifests_result_id_checker_runs FOREIGN KEY(result_id) REFERENCES public.checker_runs (result_id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_manifests_guide_id_project_guides FOREIGN KEY(guide_id) REFERENCES public.project_guides (id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_manifests_submission_binding_id_artifa_4ee3 FOREIGN KEY(submission_binding_id) REFERENCES public.artifact_bindings (id) ON DELETE RESTRICT +) +""") + op.execute(""" +CREATE TABLE public.review_packet_guide_items ( + packet_id UUID NOT NULL, + source_item_id UUID NOT NULL, + ingest_id UUID NOT NULL, + item_order INTEGER NOT NULL, + logical_role VARCHAR(32) NOT NULL, + media_type VARCHAR(100) NOT NULL, + CONSTRAINT pk_review_packet_guide_items PRIMARY KEY (packet_id, source_item_id), + CONSTRAINT uq_review_packet_ingest UNIQUE (packet_id, ingest_id), + CONSTRAINT uq_review_packet_order UNIQUE (packet_id, item_order), + CONSTRAINT ck_review_packet_guide_items_order_nonnegative CHECK (item_order >= 0), + CONSTRAINT ck_review_packet_guide_items_original_guide CHECK (logical_role='guide_source_original'), + CONSTRAINT ck_review_packet_guide_items_guide_media CHECK (media_type in ('application/pdf','application/vnd.openxmlformats-officedocument.wordprocessingml.document','application/vnd.openxmlformats-officedocument.presentationml.presentation')), + CONSTRAINT fk_review_packet_guide_items_packet_id_review_packet_manifests FOREIGN KEY(packet_id) REFERENCES public.review_packet_manifests (id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_guide_items_source_item_id_guide_sourc_211e FOREIGN KEY(source_item_id) REFERENCES public.guide_source_snapshot_items (id) ON DELETE RESTRICT, + CONSTRAINT fk_review_packet_guide_items_ingest_id_guide_source_art_22a8 FOREIGN KEY(ingest_id) REFERENCES public.guide_source_artifact_ingests (id) ON DELETE RESTRICT +) +""") + op.execute(""" +CREATE FUNCTION public.guard_review_packet_creation() RETURNS trigger +LANGUAGE plpgsql SET search_path=pg_catalog,public,pg_temp AS $$ +DECLARE lease_row public.review_leases%rowtype; queue_row public.review_queue_entries%rowtype; +BEGIN + SELECT * INTO lease_row FROM public.review_leases + WHERE id=NEW.review_lease_id AND project_id=NEW.project_id FOR UPDATE; + IF NOT FOUND THEN + RAISE EXCEPTION 'review packet lease unavailable' USING ERRCODE='23514'; + END IF; + SELECT * INTO queue_row FROM public.review_queue_entries + WHERE id=lease_row.review_queue_entry_id AND project_id=NEW.project_id FOR UPDATE; + IF NOT FOUND OR lease_row.status <> 'active' OR queue_row.queue_state <> 'leased' + OR queue_row.active_lease_id IS DISTINCT FROM lease_row.id + OR (NEW.review_queue_entry_id,NEW.task_id,NEW.submission_id,NEW.submission_version, + NEW.checker_run_id,NEW.packet_manifest_generation) IS DISTINCT FROM + (queue_row.id,lease_row.task_id,lease_row.submission_id,lease_row.submission_version, + queue_row.admitting_checker_run_id,lease_row.attempt_generation) + THEN + RAISE EXCEPTION 'review packet requires exact active lease' USING ERRCODE='23514'; + END IF; + NEW.created_at := pg_catalog.clock_timestamp(); + RETURN NEW; +END $$ +""") + op.execute(""" +CREATE TRIGGER review_packet_creation BEFORE INSERT ON public.review_packet_manifests +FOR EACH ROW EXECUTE FUNCTION public.guard_review_packet_creation() +""") + op.execute(""" +CREATE FUNCTION public.validate_review_packet(packet_uuid uuid) RETURNS void +LANGUAGE plpgsql SET search_path=pg_catalog,public,pg_temp AS $$ +DECLARE packet public.review_packet_manifests%rowtype; member_count integer; membership jsonb; +BEGIN + SELECT * INTO packet FROM public.review_packet_manifests WHERE id=packet_uuid; + IF NOT FOUND THEN + RAISE EXCEPTION 'review packet parent unavailable' USING ERRCODE='23514'; + END IF; + IF NOT EXISTS ( + SELECT 1 FROM public.submissions s + JOIN public.checker_runs r ON r.id=packet.checker_run_id + JOIN public.artifact_bindings b ON b.id=s.artifact_binding_id + JOIN public.artifact_contents c ON c.id=b.content_id + JOIN public.project_guides g ON g.id=packet.guide_id + JOIN public.guide_mutation_idempotency_records a ON a.operation_id=g.activation_operation_id + JOIN public.project_setup_runs setup ON setup.id=packet.project_setup_run_id + JOIN public.guide_source_snapshots snapshot ON snapshot.id=packet.source_snapshot_id + WHERE s.id=packet.submission_id AND s.task_id=packet.task_id AND s.version=packet.submission_version + AND s.locked_guide_version=packet.guide_version + AND s.locked_guide_source_snapshot_id=packet.source_snapshot_id + AND s.locked_guide_source_snapshot_hash=snapshot.bundle_hash + AND r.submission_id=s.id AND r.task_id=s.task_id AND r.submission_version=s.version + AND r.result_id=packet.result_id AND r.status='completed' + AND r.routing_recommendation='allow_review' + AND b.id=packet.submission_binding_id AND b.project_id=packet.project_id + AND b.resource_type='submission' AND b.resource_id=s.id::text + AND b.logical_role=packet.submission_logical_role AND b.scope_version=1 + AND c.media_type=packet.submission_media_type + AND g.project_id=packet.project_id AND g.version=packet.guide_version + AND g.status IN ('active','superseded') + AND a.project_id=g.project_id AND a.resource_id=g.id + AND a.response_json::jsonb #>> '{command,target,proposal,setup_run_id}'=setup.id::text + AND a.response_json::jsonb #>> '{command,target,proposal,setup_generation}'=setup.setup_generation::text + AND setup.source_snapshot_id=snapshot.id AND setup.guide_id=g.id + AND setup.project_id=g.project_id AND setup.guide_version=g.version + AND snapshot.guide_id=g.id AND snapshot.project_id=g.project_id AND snapshot.guide_version=g.version + ) THEN + RAISE EXCEPTION 'review packet canonical header mismatch' USING ERRCODE='23514'; + END IF; + SELECT count(*) INTO member_count FROM public.review_packet_guide_items WHERE packet_id=packet.id; + IF member_count NOT BETWEEN 1 AND 100 + OR member_count<>(SELECT count(*) FROM public.guide_source_snapshot_items WHERE source_snapshot_id=packet.source_snapshot_id) + OR EXISTS ( + SELECT 1 FROM public.review_packet_guide_items m + LEFT JOIN public.guide_source_snapshot_items item ON item.id=m.source_item_id + LEFT JOIN public.guide_source_artifact_ingests ingest ON ingest.id=m.ingest_id + WHERE m.packet_id=packet.id AND ( + item.id IS NULL OR ingest.id IS NULL OR + (item.source_snapshot_id,item.item_order,item.media_type,item.source_kind,item.ingestion_adapter, + ingest.source_item_id,ingest.media_type) IS DISTINCT FROM + (packet.source_snapshot_id,m.item_order,m.media_type,'document','upload',m.source_item_id,m.media_type) + OR NOT EXISTS ( + SELECT 1 FROM public.artifact_put_attempts put + JOIN public.artifact_replicas replica ON replica.id=put.replica_id + JOIN public.artifact_contents content ON content.id=replica.content_id + WHERE put.guide_source_item_id=item.id AND put.project_id=packet.project_id + AND put.producer_request_type='guide' AND put.logical_role IS NULL + AND put.status='object_confirmed' + AND (put.sha256,put.byte_count,put.media_type)=(ingest.sha256,ingest.byte_count,ingest.media_type) + AND (content.sha256,content.byte_count,content.media_type)=(ingest.sha256,ingest.byte_count,ingest.media_type) + AND replica.storage_namespace_id=put.storage_namespace_id + AND replica.namespace_fingerprint=put.namespace_fingerprint + AND ( + (put.terminal_result_code='document_stored' AND EXISTS ( + SELECT 1 FROM public.artifact_operation_receipts receipt + WHERE receipt.id=put.receipt_id AND receipt.put_attempt_id=put.id + AND receipt.guide_source_item_id=item.id AND receipt.replica_id=replica.id + AND receipt.request_digest=put.request_digest AND receipt.provider_object_ref=replica.provider_object_ref + AND receipt.outcome='document_stored' + )) OR + (put.terminal_result_code='document_stored_observed' AND put.receipt_id IS NULL AND EXISTS ( + SELECT 1 FROM public.artifact_put_observation_receipts receipt + WHERE receipt.put_attempt_id=put.id AND receipt.execution_generation=put.execution_generation + AND receipt.outcome='observed_confirmed' + AND receipt.expected_sha256=put.sha256 AND receipt.observed_sha256=put.sha256 + AND receipt.expected_byte_count=put.byte_count AND receipt.observed_byte_count=put.byte_count + )) + ) + ) + ) + ) THEN + RAISE EXCEPTION 'review packet canonical guide membership mismatch' USING ERRCODE='23514'; + END IF; + SELECT pg_catalog.jsonb_build_object( + 'request',pg_catalog.jsonb_build_object( + 'project_id',packet.project_id,'task_id',packet.task_id,'submission_id',packet.submission_id, + 'submission_version',packet.submission_version,'checker_run_id',packet.checker_run_id, + 'result_id',packet.result_id,'guide_id',packet.guide_id,'guide_version',packet.guide_version, + 'source_snapshot_id',packet.source_snapshot_id,'project_setup_run_id',packet.project_setup_run_id, + 'setup_generation',packet.setup_generation), + 'submission',pg_catalog.jsonb_build_object('binding_id',packet.submission_binding_id, + 'logical_role',packet.submission_logical_role,'media_type',packet.submission_media_type), + 'guide_documents',(SELECT pg_catalog.jsonb_agg(pg_catalog.jsonb_build_object( + 'ingest_id',m.ingest_id,'source_item_id',m.source_item_id,'item_order',m.item_order, + 'logical_role',m.logical_role,'media_type',m.media_type) ORDER BY m.item_order) + FROM public.review_packet_guide_items m WHERE m.packet_id=packet.id) + ) INTO membership; + IF packet.packet_manifest_digest IS DISTINCT FROM ('sha256:' || pg_catalog.encode(pg_catalog.sha256( + pg_catalog.convert_to(public.project_guide_projection_canonical_json(membership),'UTF8')),'hex')) THEN + RAISE EXCEPTION 'review packet semantic digest mismatch' USING ERRCODE='23514'; + END IF; +END $$ +""") + op.execute(""" +CREATE FUNCTION public.guard_review_packet_membership() RETURNS trigger +LANGUAGE plpgsql SET search_path=pg_catalog,public,pg_temp AS $$ +BEGIN + IF TG_TABLE_NAME='review_packet_manifests' THEN + PERFORM public.validate_review_packet(NEW.id); + ELSE + PERFORM public.validate_review_packet(NEW.packet_id); + END IF; + RETURN NEW; +END $$ +""") + op.execute(""" +CREATE CONSTRAINT TRIGGER review_packet_membership +AFTER INSERT ON public.review_packet_manifests DEFERRABLE INITIALLY DEFERRED +FOR EACH ROW EXECUTE FUNCTION public.guard_review_packet_membership() +""") + op.execute(""" +CREATE CONSTRAINT TRIGGER review_packet_member_custody +AFTER INSERT ON public.review_packet_guide_items DEFERRABLE INITIALLY DEFERRED +FOR EACH ROW EXECUTE FUNCTION public.guard_review_packet_membership() +""") + for table in ( + "review_packet_manifests", + "review_packet_guide_items", + "guide_source_artifact_ingests", + ): + op.execute(f""" +CREATE TRIGGER {table}_immutable BEFORE UPDATE OR DELETE OR TRUNCATE ON public.{table} +FOR EACH STATEMENT EXECUTE FUNCTION public.reject_artifact_fact_mutation(); +""") + + +def downgrade() -> None: + """Retained packets and source identities cannot lose custody.""" + raise RuntimeError("Workstream v0.1 migrations cannot be downgraded; recreate the database") diff --git a/backend/app/db/models.py b/backend/app/db/models.py index 2d60a76dd..e05a8f30b 100644 --- a/backend/app/db/models.py +++ b/backend/app/db/models.py @@ -92,3 +92,5 @@ ) from app.modules.projects.post_policy.models import PostPolicyOperation # noqa: F401 + +from app.modules.reviews.packet.models import ReviewPacketManifest, ReviewPacketGuideItem # noqa: F401 diff --git a/backend/app/modules/artifacts/api/review_packet.py b/backend/app/modules/artifacts/api/review_packet.py index f3dca2d89..9dc55973d 100644 --- a/backend/app/modules/artifacts/api/review_packet.py +++ b/backend/app/modules/artifacts/api/review_packet.py @@ -53,11 +53,11 @@ class ReviewSubmissionMember(BaseModel): class ReviewGuideMember(BaseModel): - """Required original document from ART's guide_source_artifact_bindings.""" + """Required original document from the live guide_source_artifact_ingests owner.""" model_config = ConfigDict(extra="forbid", frozen=True, strict=True) - guide_binding_id: UUID + ingest_id: UUID source_item_id: UUID item_order: StrictInt = Field(ge=0) logical_role: Literal["guide_source_original"] @@ -75,7 +75,7 @@ class ReviewPacketMembership(BaseModel): @model_validator(mode="after") def canonical_documents(self) -> ReviewPacketMembership: - for attribute in ("guide_binding_id", "source_item_id", "item_order"): + for attribute in ("ingest_id", "source_item_id", "item_order"): values = [getattr(document, attribute) for document in self.guide_documents] if len(values) != len(set(values)): raise ValueError("review packet guide membership is duplicated") diff --git a/backend/app/modules/projects/models.py b/backend/app/modules/projects/models.py index 1f3ca53b3..09368b01a 100644 --- a/backend/app/modules/projects/models.py +++ b/backend/app/modules/projects/models.py @@ -983,7 +983,7 @@ class GuideSourceSnapshotItem(Base): class GuideSourceArtifactIngest(Base): - """Server-owned prepared-byte facts for one not-yet-bound guide item.""" + """Immutable prepared-byte identity for one uploaded guide document.""" __tablename__ = "guide_source_artifact_ingests" __table_args__ = ( diff --git a/backend/app/modules/reviews/packet/__init__.py b/backend/app/modules/reviews/packet/__init__.py new file mode 100644 index 000000000..a494814c0 --- /dev/null +++ b/backend/app/modules/reviews/packet/__init__.py @@ -0,0 +1 @@ +"""Immutable reviewer packet metadata persistence; no runtime authority.""" diff --git a/backend/app/modules/reviews/packet/models.py b/backend/app/modules/reviews/packet/models.py new file mode 100644 index 000000000..07b654c13 --- /dev/null +++ b/backend/app/modules/reviews/packet/models.py @@ -0,0 +1,156 @@ +"""Normalized immutable reviewer packets and their exact guide members.""" + +from datetime import datetime +from uuid import UUID + +from sqlalchemy import ( + CheckConstraint, + DateTime, + ForeignKey, + ForeignKeyConstraint, + Integer, + String, + UniqueConstraint, + Uuid, + text, +) +from sqlalchemy.orm import Mapped, mapped_column + +from app.db.base import Base + + +class ReviewPacketManifest(Base): + """One retained metadata packet per lease, with exactly one original ZIP.""" + + __tablename__ = "review_packet_manifests" + __table_args__ = ( + CheckConstraint( + "(get_byte(uuid_send(id), 6) >> 4) = 7 and (get_byte(uuid_send(id), 8) & 192) = 128", + name="id_uuid7", + ), + UniqueConstraint("review_lease_id", name="uq_review_packet_lease"), + ForeignKeyConstraint( + ["task_id", "project_id"], + ["workstream_tasks.id", "workstream_tasks.project_id"], + ondelete="RESTRICT", + name="fk_review_packet_task", + ), + ForeignKeyConstraint( + ["submission_id", "task_id", "submission_version"], + ["submissions.id", "submissions.task_id", "submissions.version"], + ondelete="RESTRICT", + name="fk_review_packet_submission", + ), + ForeignKeyConstraint( + ["checker_run_id", "task_id", "submission_id"], + ["checker_runs.id", "checker_runs.task_id", "checker_runs.submission_id"], + ondelete="RESTRICT", + name="fk_review_packet_checker", + ), + ForeignKeyConstraint( + ["source_snapshot_id", "project_id", "guide_id"], + [ + "guide_source_snapshots.id", + "guide_source_snapshots.project_id", + "guide_source_snapshots.guide_id", + ], + ondelete="RESTRICT", + name="fk_review_packet_snapshot", + ), + ForeignKeyConstraint( + [ + "project_setup_run_id", + "project_id", + "guide_id", + "source_snapshot_id", + "setup_generation", + ], + [ + "project_setup_runs.id", + "project_setup_runs.project_id", + "project_setup_runs.guide_id", + "project_setup_runs.source_snapshot_id", + "project_setup_runs.setup_generation", + ], + ondelete="RESTRICT", + name="fk_review_packet_setup", + ), + CheckConstraint( + "submission_version > 0 and setup_generation > 0 and packet_manifest_generation > 0", + name="positive_generations", + ), + CheckConstraint("length(btrim(guide_version)) > 0", name="guide_version_nonblank"), + CheckConstraint("packet_manifest_digest ~ '^sha256:[0-9a-f]{64}$'", name="digest_shape"), + CheckConstraint( + "submission_logical_role='submission_bundle_original' and submission_media_type='application/zip'", + name="original_zip", + ), + ) + + id: Mapped[UUID] = mapped_column(Uuid(), primary_key=True) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=text("clock_timestamp()") + ) + review_lease_id: Mapped[UUID] = mapped_column( + Uuid(), ForeignKey("review_leases.id", ondelete="RESTRICT"), nullable=False + ) + review_queue_entry_id: Mapped[UUID] = mapped_column( + Uuid(), ForeignKey("review_queue_entries.id", ondelete="RESTRICT"), nullable=False + ) + packet_manifest_generation: Mapped[int] = mapped_column(Integer, nullable=False) + packet_manifest_digest: Mapped[str] = mapped_column(String(71), nullable=False) + project_id: Mapped[str] = mapped_column(Uuid(as_uuid=False), nullable=False) + task_id: Mapped[str] = mapped_column(Uuid(as_uuid=False), nullable=False) + submission_id: Mapped[str] = mapped_column(Uuid(as_uuid=False), nullable=False) + submission_version: Mapped[int] = mapped_column(Integer, nullable=False) + checker_run_id: Mapped[str] = mapped_column(Uuid(as_uuid=False), nullable=False) + result_id: Mapped[str] = mapped_column( + Uuid(as_uuid=False), + ForeignKey("checker_runs.result_id", ondelete="RESTRICT"), + nullable=False, + ) + guide_id: Mapped[str] = mapped_column( + Uuid(as_uuid=False), ForeignKey("project_guides.id", ondelete="RESTRICT"), nullable=False + ) + guide_version: Mapped[str] = mapped_column(String(50), nullable=False) + source_snapshot_id: Mapped[str] = mapped_column(Uuid(as_uuid=False), nullable=False) + project_setup_run_id: Mapped[str] = mapped_column(Uuid(as_uuid=False), nullable=False) + setup_generation: Mapped[int] = mapped_column(Integer, nullable=False) + submission_binding_id: Mapped[str] = mapped_column( + Uuid(as_uuid=False), ForeignKey("artifact_bindings.id", ondelete="RESTRICT"), nullable=False + ) + submission_logical_role: Mapped[str] = mapped_column(String(32), nullable=False) + submission_media_type: Mapped[str] = mapped_column(String(100), nullable=False) + + +class ReviewPacketGuideItem(Base): + """One normalized member of the exact declared guide document set.""" + + __tablename__ = "review_packet_guide_items" + __table_args__ = ( + UniqueConstraint("packet_id", "ingest_id", name="uq_review_packet_ingest"), + UniqueConstraint("packet_id", "item_order", name="uq_review_packet_order"), + CheckConstraint("item_order >= 0", name="order_nonnegative"), + CheckConstraint("logical_role='guide_source_original'", name="original_guide"), + CheckConstraint( + "media_type in ('application/pdf','application/vnd.openxmlformats-officedocument.wordprocessingml.document','application/vnd.openxmlformats-officedocument.presentationml.presentation')", + name="guide_media", + ), + ) + + packet_id: Mapped[UUID] = mapped_column( + Uuid(), ForeignKey("review_packet_manifests.id", ondelete="RESTRICT"), primary_key=True + ) + source_item_id: Mapped[str] = mapped_column( + Uuid(as_uuid=False), + ForeignKey("guide_source_snapshot_items.id", ondelete="RESTRICT"), + primary_key=True, + ) + ingest_id: Mapped[str] = mapped_column( + Uuid(as_uuid=False), + ForeignKey("guide_source_artifact_ingests.id", ondelete="RESTRICT"), + nullable=False, + ) + item_order: Mapped[int] = mapped_column(Integer, nullable=False) + logical_role: Mapped[str] = mapped_column(String(32), nullable=False) + media_type: Mapped[str] = mapped_column(String(100), nullable=False) diff --git a/backend/app/modules/reviews/packet/repository.py b/backend/app/modules/reviews/packet/repository.py new file mode 100644 index 000000000..ab6ea71b9 --- /dev/null +++ b/backend/app/modules/reviews/packet/repository.py @@ -0,0 +1,155 @@ +"""Caller-owned packet persistence; authorization and byte access stay separate.""" + +from uuid import UUID + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.core.hashing import canonical_json_hash +from app.core.identifiers import new_record_id +from app.modules.artifacts.api.review_packet import ( + ReviewGuideMember, + ReviewPacketMembership, + ReviewPacketMembershipRequest, + ReviewPacketMembershipUnavailable, + ReviewSubmissionMember, +) +from app.modules.reviews.models import ReviewLease, ReviewQueueEntry +from app.modules.reviews.packet.models import ReviewPacketGuideItem, ReviewPacketManifest +from app.modules.reviews.packet.schemas import ReviewPacketConflict, ReviewPacketStored + + +class ReviewPacketRepository: + """Freeze one normalized packet per lease without committing or authorizing.""" + + def __init__(self, session: AsyncSession) -> None: + self._session = session + + async def store(self, lease_id: UUID, membership: ReviewPacketMembership) -> ReviewPacketStored: + project_id = membership.request.project_id + lease = await self._session.scalar( + select(ReviewLease) + .where( + ReviewLease.id == lease_id, + ReviewLease.project_id == str(project_id), + ) + .with_for_update() + .execution_options(populate_existing=True) + ) + if lease is None: + raise ReviewPacketMembershipUnavailable() + queue = await self._session.scalar( + select(ReviewQueueEntry) + .where( + ReviewQueueEntry.id == lease.review_queue_entry_id, + ReviewQueueEntry.project_id == str(project_id), + ) + .with_for_update() + .execution_options(populate_existing=True) + ) + existing = await self.read(project_id, lease_id) + if existing is not None: + if existing.membership != membership: + raise ReviewPacketConflict() + return existing + if ( + queue is None + or lease.status != "active" + or queue.queue_state != "leased" + or queue.active_lease_id != lease.id + ): + raise ReviewPacketMembershipUnavailable() + packet = ReviewPacketManifest( + id=new_record_id(), + review_lease_id=lease_id, + review_queue_entry_id=queue.id, + packet_manifest_generation=lease.attempt_generation, + packet_manifest_digest=canonical_json_hash(membership.model_dump(mode="json")), + **{ + key: str(value) if isinstance(value, UUID) else value + for key, value in membership.request.model_dump().items() + }, + submission_binding_id=str(membership.submission.binding_id), + submission_logical_role=membership.submission.logical_role, + submission_media_type=membership.submission.media_type, + ) + self._session.add(packet) + await self._session.flush() + self._session.add_all( + [ + ReviewPacketGuideItem( + packet_id=packet.id, + **{ + key: str(value) if isinstance(value, UUID) else value + for key, value in member.model_dump().items() + }, + ) + for member in membership.guide_documents + ] + ) + await self._session.flush() + return ReviewPacketStored( + packet_id=packet.id, + review_lease_id=lease_id, + review_queue_entry_id=queue.id, + packet_manifest_generation=packet.packet_manifest_generation, + packet_manifest_digest=packet.packet_manifest_digest, + created_at=packet.created_at, + membership=membership, + ) + + async def read(self, project_id: UUID, lease_id: UUID) -> ReviewPacketStored | None: + """Project-qualified metadata only; no current authority or availability claim.""" + packet = await self._session.scalar( + select(ReviewPacketManifest) + .where( + ReviewPacketManifest.project_id == str(project_id), + ReviewPacketManifest.review_lease_id == lease_id, + ) + .execution_options(populate_existing=True) + ) + if packet is None: + return None + members = ( + await self._session.scalars( + select(ReviewPacketGuideItem) + .where( + ReviewPacketGuideItem.packet_id == packet.id, + ) + .order_by(ReviewPacketGuideItem.item_order) + ) + ).all() + return ReviewPacketStored( + packet_id=packet.id, + review_lease_id=packet.review_lease_id, + review_queue_entry_id=packet.review_queue_entry_id, + packet_manifest_generation=packet.packet_manifest_generation, + packet_manifest_digest=packet.packet_manifest_digest, + created_at=packet.created_at, + membership=ReviewPacketMembership( + request=ReviewPacketMembershipRequest( + **{ + key: UUID(str(getattr(packet, key))) + if key.endswith("_id") + else getattr(packet, key) + for key in ReviewPacketMembershipRequest.model_fields + } + ), + submission=ReviewSubmissionMember( + binding_id=UUID(packet.submission_binding_id), + logical_role=packet.submission_logical_role, + media_type=packet.submission_media_type, + ), + guide_documents=tuple( + ReviewGuideMember( + **{ + key: UUID(str(getattr(member, key))) + if key.endswith("_id") + else getattr(member, key) + for key in ReviewGuideMember.model_fields + } + ) + for member in members + ), + ), + ) diff --git a/backend/app/modules/reviews/packet/schemas.py b/backend/app/modules/reviews/packet/schemas.py new file mode 100644 index 000000000..f2b7e3d09 --- /dev/null +++ b/backend/app/modules/reviews/packet/schemas.py @@ -0,0 +1,28 @@ +"""Detached packet facts, without authority or storage capabilities.""" + +from uuid import UUID + +from pydantic import AwareDatetime, BaseModel, ConfigDict, Field, StrictInt + +from app.modules.artifacts.api.review_packet import ReviewPacketMembership + + +class ReviewPacketConflict(RuntimeError): + """The same lease already retains different packet membership.""" + + def __init__(self) -> None: + super().__init__("review_packet_conflict") + + +class ReviewPacketStored(BaseModel): + """Immutable stored identity plus canonical ART metadata shape.""" + + model_config = ConfigDict(extra="forbid", frozen=True, strict=True) + + packet_id: UUID + review_lease_id: UUID + review_queue_entry_id: UUID + packet_manifest_generation: StrictInt = Field(ge=1) + packet_manifest_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") + created_at: AwareDatetime + membership: ReviewPacketMembership diff --git a/backend/scripts/behavior_ownership.py b/backend/scripts/behavior_ownership.py index aa09fb43b..6a6ab88d5 100644 --- a/backend/scripts/behavior_ownership.py +++ b/backend/scripts/behavior_ownership.py @@ -147,6 +147,7 @@ "backend/app/modules/authorization/api/outbox_dispatch.py", } ) +REV_03B_PACKET_TARGETS = frozenset({'backend/app/modules/reviews/packet/models.py', 'backend/app/modules/reviews/packet/repository.py', 'backend/app/modules/reviews/packet/schemas.py'}) ARCH_04E1A_SOURCE_TARGETS = frozenset({ "backend/app/modules/tasks/api/accepted_effects.py", "backend/app/modules/tasks/api/post_submit_routing.py", @@ -738,6 +739,7 @@ def _validate_additive_partition_transition( | ARCH_CP03B_ADAPTER_BINDING_AUTH_TARGETS | ARCH_CP04A_CONTRIBUTION_POLICY_TARGETS | ARCH_CP04B_CONTRIBUTION_POLICY_TARGETS + | REV_03B_PACKET_TARGETS | ARCH_04E1A_SOURCE_TARGETS | ARCH_04D2_AUTHORITY_TARGETS | ARCH_04C_EXECUTION_TARGETS diff --git a/backend/scripts/identifier_inventory.py b/backend/scripts/identifier_inventory.py index 48e69ef9d..5ff900ce7 100644 --- a/backend/scripts/identifier_inventory.py +++ b/backend/scripts/identifier_inventory.py @@ -20,6 +20,7 @@ GENERATION_CLASSIFICATION_FORMAT = "workstream-uuid-generation-classifications-1" GENERATION_CLASSIFICATIONS = "identifier_generation_classifications.json" SEMANTIC_KEYS = { + "review_packet_guide_items": ("packet-to-declared-source membership", ("packet_id", "source_item_id")), "actor_profile_migration_state": ("seeded schema-state singleton, not a record sequence", ("id",)), "api_rate_control_counters": ("rate-limit scope and digest", ("control_scope", "key_digest")), "artifact_admission_scopes": ("artifact quota scope", ("scope_type", "scope_id")), diff --git a/backend/scripts/test_lane_catalogue.py b/backend/scripts/test_lane_catalogue.py index f37c2b090..72a1fc65a 100644 --- a/backend/scripts/test_lane_catalogue.py +++ b/backend/scripts/test_lane_catalogue.py @@ -368,6 +368,10 @@ class TestLane: ) TASK_MODULES = ( + "tests/reviews/packet/test_repository.py", + "tests/reviews/packet/test_storage.py", + "tests/reviews/packet/test_migration.py", + "tests/tasks/post_submit_routing/test_contracts.py", "tests/tasks/post_submit_routing/test_storage.py", "tests/tasks/post_submit_routing/test_migration.py", diff --git a/backend/tests/artifacts/test_review_packet_contract.py b/backend/tests/artifacts/test_review_packet_contract.py index 605e64231..66a4a6431 100644 --- a/backend/tests/artifacts/test_review_packet_contract.py +++ b/backend/tests/artifacts/test_review_packet_contract.py @@ -36,7 +36,7 @@ def packet() -> dict: def guide(order: int) -> dict: return { - "guide_binding_id": UUID(int=100 + order), + "ingest_id": UUID(int=100 + order), "source_item_id": UUID(int=300 + order), "item_order": order, "logical_role": "guide_source_original", "media_type": "application/pdf", } @@ -69,7 +69,7 @@ def test_closed_public_field_inventories() -> None: } assert set(ReviewSubmissionMember.model_fields) == {"binding_id", "logical_role", "media_type"} assert set(ReviewGuideMember.model_fields) == { - "guide_binding_id", "source_item_id", "item_order", "logical_role", "media_type", + "ingest_id", "source_item_id", "item_order", "logical_role", "media_type", } assert set(ReviewPacketMembership.model_fields) == {"request", "submission", "guide_documents"} with pytest.raises(TypeError, match="Protocols cannot be instantiated"): @@ -120,7 +120,7 @@ def test_invalid_ids_and_strict_versions() -> None: ReviewPacketMembershipRequest.model_validate({**raw["request"], name: "invalid"}) for model, data, fields in ( (ReviewSubmissionMember, raw["submission"], ["binding_id"]), - (ReviewGuideMember, raw["guide_documents"][0], ["guide_binding_id", "source_item_id"]), + (ReviewGuideMember, raw["guide_documents"][0], ["ingest_id", "source_item_id"]), ): for name in fields: with pytest.raises(ValidationError): @@ -151,7 +151,7 @@ def test_closed_roles_and_supported_media() -> None: assert ReviewGuideMember.model_validate({**guide(0), "media_type": media}).media_type == media -@pytest.mark.parametrize("attribute", ["guide_binding_id", "source_item_id", "item_order"]) +@pytest.mark.parametrize("attribute", ["ingest_id", "source_item_id", "item_order"]) def test_independent_duplicate_membership_rejected(attribute: str) -> None: raw = packet() raw["guide_documents"][1][attribute] = raw["guide_documents"][0][attribute] @@ -184,7 +184,7 @@ def test_python_uuid_strings_rejected_at_each_nested_boundary() -> None: ("request", ("project_id", "task_id", "submission_id", "checker_run_id", "result_id", "guide_id", "source_snapshot_id", "project_setup_run_id")), ("submission", ("binding_id",)), - ("guide", ("guide_binding_id", "source_item_id")), + ("guide", ("ingest_id", "source_item_id")), ): for field in fields: changed = deepcopy(raw) @@ -196,3 +196,11 @@ def test_python_uuid_strings_rejected_at_each_nested_boundary() -> None: assert caught.value.errors()[0]["type"] == "is_instance_of" expected_location = ("guide_documents", 0, field) if scope == "guide" else (scope, field) assert caught.value.errors()[0]["loc"] == expected_location + + +def test_removed_guide_binding_identity_is_not_an_alias(): + from app.modules.artifacts.api.review_packet import ReviewGuideMember + from app.core.identifiers import new_record_id + with pytest.raises(ValidationError): + ReviewGuideMember(guide_binding_id=new_record_id(),source_item_id=new_record_id(), + item_order=0,logical_role='guide_source_original',media_type='application/pdf') diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index de18d9edb..77abf2256 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -23,7 +23,7 @@ DDL_LOCK_DIRECTORY = Path("/tmp") # Match the PostgreSQL 16 engine used by Backend CI. Catalog identity rendering # differs across major versions; regenerate only after comparing actual objects. -EXPECTED_PUBLIC_SCHEMA_SHA256 = "d9c94fbdce2a2fa9aa822b9e0f528f4497362ed0c6b2aef1946f454ea250b354" +EXPECTED_PUBLIC_SCHEMA_SHA256 = "7b79a9331aad5d7fc19fe85fa8c86f8effea39ccdb7c1ed860c1bbdf60098868" PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", "alembic_version", @@ -108,6 +108,8 @@ "review_policies", "revision_policies", "review_admission_idempotency_records", + "review_packet_manifests", + "review_packet_guide_items", "review_leases", "review_queue_entries", "submission_policy_mutation_idempotency_records", @@ -141,6 +143,7 @@ "guide_sufficiency_report_source_usages", "guide_sufficiency_mutation_idempotency_records", "guide_source_snapshot_items", + "guide_source_artifact_ingests", "guide_source_artifact_bindings", "guide_source_format_classifications", "guide_source_extraction_attempts", @@ -174,6 +177,8 @@ "project_role_grants", "project_role_qualification_snapshots", "review_admission_idempotency_records", + "review_packet_manifests", + "review_packet_guide_items", "review_leases", "review_queue_entries", "review_policies", diff --git a/backend/tests/reviews/packet/__init__.py b/backend/tests/reviews/packet/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/tests/reviews/packet/support.py b/backend/tests/reviews/packet/support.py new file mode 100644 index 000000000..e3e8544fc --- /dev/null +++ b/backend/tests/reviews/packet/support.py @@ -0,0 +1,184 @@ +"""Packets built from real activated guides, admitted ZIPs and executed checker runs.""" + +from contextlib import asynccontextmanager +from datetime import UTC, datetime, timedelta +from uuid import UUID + +from sqlalchemy import select, text + +from app.core.identifiers import new_record_id +from app.modules.artifacts.api.review_packet import ( + ReviewGuideMember, + ReviewPacketMembership, + ReviewPacketMembershipRequest, + ReviewSubmissionMember, +) +from app.modules.projects.models import ( + GuideMutationIdempotencyRecord, + GuideSourceArtifactIngest, + GuideSourceSnapshotItem, + ProjectGuide, +) +from app.modules.reviews.repository import ReviewQueueRepository +from app.modules.reviews.schemas import ( + ReviewLeaseInput, + ReviewQueueEntryInput, + ReviewRoutingMode, + ReviewRoutingReason, +) +from app.modules.tasks.models import Submission +from tests.tasks.post_submit_routing.support import completed_source +from tests.test_review_lease_persistence import _human_actor + + +@asynccontextmanager +async def packet_source(tmp_path, database_url, **source_options): + async with completed_source(tmp_path, database_url, **source_options) as h: + async with h.factory() as session: + submission = await session.get(Submission, str(h.request.submission_id)) + guide = await session.scalar( + select(ProjectGuide).where( + ProjectGuide.project_id == str(h.source["project_id"]), + ProjectGuide.version == submission.locked_guide_version, + ) + ) + activation = await session.scalar( + select(GuideMutationIdempotencyRecord).where( + GuideMutationIdempotencyRecord.operation_id == guide.activation_operation_id, + ) + ) + proposal = activation.response_json["command"]["target"]["proposal"] + rows = ( + await session.execute( + select(GuideSourceSnapshotItem, GuideSourceArtifactIngest) + .join( + GuideSourceArtifactIngest, + GuideSourceArtifactIngest.source_item_id == GuideSourceSnapshotItem.id, + ) + .where( + GuideSourceSnapshotItem.source_snapshot_id + == submission.locked_guide_source_snapshot_id + ) + .order_by(GuideSourceSnapshotItem.item_order) + ) + ).all() + h.membership = ReviewPacketMembership( + request=ReviewPacketMembershipRequest( + project_id=h.source["project_id"], + task_id=h.request.task_id, + submission_id=h.request.submission_id, + submission_version=submission.version, + checker_run_id=h.result.attempt_id, + result_id=h.source["result_id"], + guide_id=UUID(guide.id), + guide_version=guide.version, + source_snapshot_id=UUID(submission.locked_guide_source_snapshot_id), + project_setup_run_id=UUID(proposal["setup_run_id"]), + setup_generation=proposal["setup_generation"], + ), + submission=ReviewSubmissionMember( + binding_id=UUID(submission.artifact_binding_id), + logical_role="submission_bundle_original", + media_type="application/zip", + ), + guide_documents=tuple( + ReviewGuideMember( + ingest_id=UUID(ingest.id), + source_item_id=UUID(item.id), + item_order=item.item_order, + logical_role="guide_source_original", + media_type=item.media_type, + ) + for item, ingest in rows + ), + ) + repository = ReviewQueueRepository(session) + queue = await repository.add_queue_entry( + ReviewQueueEntryInput( + id=new_record_id(), + project_id=str(h.source["project_id"]), + task_id=str(h.request.task_id), + submission_id=submission.id, + submission_version=submission.version, + admitting_checker_run_id=str(h.result.attempt_id), + routing_mode=ReviewRoutingMode.OPEN, + routing_reason=ReviewRoutingReason.FIRST_SUBMISSION, + ) + ) + await session.commit() + actor_id = await _human_actor(session, label="packet-reviewer") + lease = await repository.add_lease( + ReviewLeaseInput( + id=new_record_id(), + review_queue_entry_id=queue.id, + project_id=queue.project_id, + task_id=queue.task_id, + submission_id=submission.id, + submission_version=submission.version, + reviewer_id=actor_id, + reviewer_contribution_policy_version_id=h.source[ + "contribution_policy_version_id" + ], + attempt_generation=1, + expires_at=datetime.now(UTC) + timedelta(days=1), + ) + ) + queue.queue_state = "leased" + queue.active_lease_id = lease.id + await session.commit() + h.lease_id, h.queue_id = lease.id, queue.id + yield h + + +async def raw_insert(session, h, *, header=None, members=None): + """SQL insertion bypassing repository validation; defer canonical reconciliation.""" + from app.core.hashing import canonical_json_hash + + values = dict( + id=new_record_id(), + review_lease_id=h.lease_id, + review_queue_entry_id=h.queue_id, + packet_manifest_generation=1, + packet_manifest_digest=canonical_json_hash(h.membership.model_dump(mode="json")), + **h.membership.request.model_dump(), + submission_binding_id=h.membership.submission.binding_id, + submission_logical_role=h.membership.submission.logical_role, + submission_media_type=h.membership.submission.media_type, + ) + import json + + actual_members = h.membership.guide_documents if members is None else members + body = h.membership.model_dump(mode="json") + body["guide_documents"] = [ + m.model_dump(mode="json") if hasattr(m, "model_dump") else m for m in actual_members + ] + values["packet_manifest_digest"] = canonical_json_hash( + json.loads(json.dumps(body, default=str)) + ) + values.update(header or {}) + await session.execute( + text( + "INSERT INTO public.review_packet_manifests (" + + ",".join(values) + + ") VALUES (" + + ",".join(":" + key for key in values) + + ")" + ), + values, + ) + for member in h.membership.guide_documents if members is None else members: + item = dict( + packet_id=values["id"], + **(member.model_dump() if hasattr(member, "model_dump") else member), + ) + await session.execute( + text( + "INSERT INTO public.review_packet_guide_items (" + + ",".join(item) + + ") VALUES (" + + ",".join(":" + key for key in item) + + ")" + ), + item, + ) + return values["id"] diff --git a/backend/tests/reviews/packet/test_migration.py b/backend/tests/reviews/packet/test_migration.py new file mode 100644 index 000000000..670f00de5 --- /dev/null +++ b/backend/tests/reviews/packet/test_migration.py @@ -0,0 +1,75 @@ +"""Populated predecessor upgrade preserves canonical owners without inventing packets.""" + +import asyncio + +import asyncpg +import pytest +from alembic import command + +from app.db import session as db_session +from tests.migration_fixtures import _config +from tests.reviews.packet.support import packet_source + +pytestmark = pytest.mark.postgres_schema_contract + + +async def snapshot(connection): + return { + table: await connection.fetch( + f"SELECT to_jsonb(r)::text AS value FROM public.{table} r ORDER BY 1" + ) + for table in ( + "submissions", + "checker_runs", + "checker_results", + "artifact_bindings", + "artifact_contents", + "guide_source_artifact_ingests", + "project_guides", + "guide_source_snapshot_items", + "review_queue_entries", + "review_leases", + ) + } + + +async def test_packet_upgrade_preserves_existing_owners( + tmp_path, isolated_database_env, migration_lock +): + with migration_lock(): + await db_session.dispose_engine() + url = isolated_database_env.replace("+asyncpg", "") + connection = await asyncpg.connect(url) + try: + await connection.execute("DROP SCHEMA public CASCADE; CREATE SCHEMA public") + finally: + await connection.close() + await asyncio.to_thread(command.upgrade, _config(), "0011_task_routing_source") + async with packet_source(tmp_path, isolated_database_env): + connection = await asyncpg.connect(url) + try: + before = await snapshot(connection) + assert ( + await connection.fetchval( + "SELECT to_regclass('public.review_packet_manifests')" + ) + is None + ) + finally: + await connection.close() + await asyncio.to_thread(command.upgrade, _config(), "0012_review_packet") + connection = await asyncpg.connect(url) + try: + assert await snapshot(connection) == before + assert ( + await connection.fetchval("SELECT count(*) FROM public.review_packet_manifests") + == 0 + ) + assert ( + await connection.fetchval( + "SELECT count(*) FROM public.review_packet_guide_items" + ) + == 0 + ) + finally: + await connection.close() diff --git a/backend/tests/reviews/packet/test_repository.py b/backend/tests/reviews/packet/test_repository.py new file mode 100644 index 000000000..4a0b780d1 --- /dev/null +++ b/backend/tests/reviews/packet/test_repository.py @@ -0,0 +1,261 @@ +"""Observable packet persistence, replay and caller transaction guarantees.""" + +import pytest +from sqlalchemy import text + +from app.core.hashing import canonical_json_hash +from app.core.identifiers import new_record_id +from app.modules.reviews.packet.repository import ReviewPacketRepository +from app.modules.reviews.packet.schemas import ReviewPacketConflict +from tests.reviews.packet.support import packet_source + + +@pytest.mark.asyncio +async def test_packet_matches_canonical_owners_and_digest(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + async with h.factory() as session: + repository = ReviewPacketRepository(session) + stored = await repository.store(h.lease_id, h.membership) + await session.commit() + assert set(stored.model_dump()) == { + "packet_id", + "review_lease_id", + "review_queue_entry_id", + "packet_manifest_generation", + "packet_manifest_digest", + "created_at", + "membership", + } + assert stored.membership == h.membership + assert stored.packet_manifest_digest == canonical_json_hash( + h.membership.model_dump(mode="json") + ) + assert stored.packet_manifest_generation == 1 + assert stored.review_queue_entry_id == h.queue_id + assert stored.review_lease_id == h.lease_id + assert await repository.read(h.membership.request.project_id, h.lease_id) == stored + assert await repository.store(h.lease_id, h.membership) == stored + changed = h.membership.model_copy( + update={ + "submission": h.membership.submission.model_copy( + update={"binding_id": new_record_id()} + ) + } + ) + with pytest.raises(ReviewPacketConflict): + await repository.store(h.lease_id, changed) + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) + == 1 + ) + assert await session.scalar( + text("SELECT count(*) FROM public.review_packet_guide_items") + ) == len(h.membership.guide_documents) + + +@pytest.mark.asyncio +async def test_packet_read_conceals_foreign_project(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + async with h.factory() as session: + await ReviewPacketRepository(session).store(h.lease_id, h.membership) + await session.commit() + async with h.factory() as session: + assert await ReviewPacketRepository(session).read(new_record_id(), h.lease_id) is None + + +@pytest.mark.asyncio +async def test_packet_caller_rollback(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + async with h.factory() as session: + await ReviewPacketRepository(session).store(h.lease_id, h.membership) + await session.rollback() + async with h.factory() as session: + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) + == 0 + ) + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_guide_items")) + == 0 + ) + + +async def wait_for_blocker(factory, pid): + import asyncio + + async def observe(): + async with factory() as session: + while not await session.scalar( + text("SELECT cardinality(pg_blocking_pids(:pid))>0"), {"pid": pid} + ): + await asyncio.sleep(0.01) + + await asyncio.wait_for(observe(), 10) + + +@pytest.mark.asyncio +async def test_packet_same_lease_concurrency(tmp_path, clean_postgres_database): + import asyncio + + async with packet_source(tmp_path, clean_postgres_database) as h: + for conflicting in (False, True): + ready = asyncio.Future() + + async def second(): + async with h.factory() as session: + ready.set_result(await session.scalar(text("SELECT pg_backend_pid()"))) + membership = h.membership + if conflicting: + membership = membership.model_copy( + update={ + "submission": membership.submission.model_copy( + update={"binding_id": new_record_id()} + ) + } + ) + result = await ReviewPacketRepository(session).store(h.lease_id, membership) + await session.commit() + return result + + async with h.factory() as first: + original = await ReviewPacketRepository(first).store(h.lease_id, h.membership) + task = asyncio.create_task(second()) + try: + await wait_for_blocker(h.factory, await ready) + await first.commit() + if conflicting: + with pytest.raises(ReviewPacketConflict): + await asyncio.wait_for(task, 10) + else: + assert await asyncio.wait_for(task, 10) == original + finally: + if not task.done(): + task.cancel() + await asyncio.gather(task, return_exceptions=True) + async with h.factory() as session: + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) + == 1 + ) + + await session.execute( + text( + "UPDATE public.review_leases SET status='released',close_reason='manual_release',closed_at=clock_timestamp() WHERE id=:id" + ), + {"id": h.lease_id}, + ) + await session.execute( + text( + "UPDATE public.review_queue_entries SET queue_state='pending',active_lease_id=NULL WHERE id=:id" + ), + {"id": h.queue_id}, + ) + await session.commit() + assert await ReviewPacketRepository(session).store(h.lease_id, h.membership) == original + await session.commit() + + +@pytest.mark.asyncio +async def test_packet_creation_serializes_with_lease_closure(tmp_path, clean_postgres_database): + import asyncio + from app.modules.artifacts.api.review_packet import ReviewPacketMembershipUnavailable + + async with packet_source(tmp_path, clean_postgres_database) as h: + ready = asyncio.Future() + + async def store(): + async with h.factory() as session: + ready.set_result(await session.scalar(text("SELECT pg_backend_pid()"))) + return await ReviewPacketRepository(session).store(h.lease_id, h.membership) + + async with h.factory() as first: + await first.execute( + text( + "UPDATE public.review_leases SET status='released',close_reason='manual_release',closed_at=clock_timestamp() WHERE id=:id" + ), + {"id": h.lease_id}, + ) + await first.execute( + text( + "UPDATE public.review_queue_entries SET queue_state='pending',active_lease_id=NULL WHERE id=:id" + ), + {"id": h.queue_id}, + ) + task = asyncio.create_task(store()) + try: + await wait_for_blocker(h.factory, await ready) + await first.commit() + with pytest.raises(ReviewPacketMembershipUnavailable): + await asyncio.wait_for(task, 10) + finally: + if not task.done(): + task.cancel() + await asyncio.gather(task, return_exceptions=True) + async with h.factory() as session: + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) + == 0 + ) + + +@pytest.mark.asyncio +async def test_packet_retains_superseded_guide_lineage(tmp_path, clean_postgres_database): + from uuid import UUID + from sqlalchemy import select + from app.modules.actors.models import ActorIdentityLink + from app.modules.authorization.models import AdminRoleGrant + from app.modules.authorization.api import ActorIdentityFacts, ActorKind + from app.modules.projects.models import ProjectGuide + from app.modules.projects.guide_activation.custody import load_guide_activation + from tests.projects.guide_activation.test_successor import successor_command + from tests.projects.guide_activation.pg_support import publish_policy + from tests.authorization.guide_activation.pg_support import activate + + async with packet_source(tmp_path, clean_postgres_database) as h: + async with h.factory() as session: + original = await ReviewPacketRepository(session).store(h.lease_id, h.membership) + await session.commit() + guide = await session.get(ProjectGuide, str(h.membership.request.guide_id)) + first = await load_guide_activation(session, guide) + grant, link = ( + await session.execute( + select(AdminRoleGrant, ActorIdentityLink) + .join( + ActorIdentityLink, + ActorIdentityLink.actor_profile_id + == AdminRoleGrant.target_actor_profile_id, + ) + .where( + AdminRoleGrant.scope_project_id == str(h.membership.request.project_id), + AdminRoleGrant.role == "project_manager", + AdminRoleGrant.status == "active", + ActorIdentityLink.status == "active", + ) + .order_by(AdminRoleGrant.id, ActorIdentityLink.id) + .limit(1) + ) + ).one() + actor = ActorIdentityFacts( + UUID(grant.target_actor_profile_id), UUID(link.id), ActorKind.HUMAN + ) + _, policy = await publish_policy(h.factory, h.membership.request.project_id) + command = await successor_command(h.factory, first.command, actor, grant.id, policy) + command = command.model_copy( + update={ + "expected_previous_active_guide_id": h.membership.request.guide_id, + "expected_previous_active_guide_generation": first.activation_generation, + } + ) + successor = await activate(h.factory, actor, command) + assert successor.command.target.proposal.guide_id != h.membership.request.guide_id + async with h.factory() as session: + guide = await session.get(ProjectGuide, str(h.membership.request.guide_id)) + assert guide.status == "superseded" + assert ( + await ReviewPacketRepository(session).read( + h.membership.request.project_id, h.lease_id + ) + == original + ) + assert await ReviewPacketRepository(session).store(h.lease_id, h.membership) == original + await session.commit() diff --git a/backend/tests/reviews/packet/test_storage.py b/backend/tests/reviews/packet/test_storage.py new file mode 100644 index 000000000..f2d858739 --- /dev/null +++ b/backend/tests/reviews/packet/test_storage.py @@ -0,0 +1,532 @@ +"""Direct PostgreSQL custody proofs independent of the packet repository.""" + +from datetime import UTC, datetime + +import pytest +from sqlalchemy import text +from sqlalchemy.exc import DBAPIError + +from tests.reviews.packet.support import packet_source, raw_insert + + +async def valid_control(h): + async with h.factory() as session: + await raw_insert(session, h) + await session.execute(text("SET CONSTRAINTS ALL IMMEDIATE")) + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_rejects_null_source_fields(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + fields = tuple(type(h.membership.request).model_fields) + ( + "review_lease_id", + "review_queue_entry_id", + "packet_manifest_generation", + "packet_manifest_digest", + "submission_binding_id", + "submission_logical_role", + "submission_media_type", + ) + for field in fields: + async with h.factory() as session: + with pytest.raises(DBAPIError): + await raw_insert(session, h, header={field: None}) + await session.commit() + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_creation_time_is_database_owned(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + for supplied in (None, datetime(2000, 1, 1, tzinfo=UTC), datetime(2100, 1, 1, tzinfo=UTC)): + async with h.factory() as session: + start = await session.scalar(text("SELECT clock_timestamp()")) + packet_id = await raw_insert(session, h, header={"created_at": supplied}) + await session.execute(text("SET CONSTRAINTS ALL IMMEDIATE")) + stored = await session.scalar( + text("SELECT created_at FROM public.review_packet_manifests WHERE id=:id"), + {"id": packet_id}, + ) + end = await session.scalar(text("SELECT clock_timestamp()")) + assert start <= stored <= end + assert stored != supplied + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_requires_complete_canonical_guide_set(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + members = list(h.membership.guide_documents) + assert len(members) == 2 + variants = [ + [], + members[:1], + list(reversed(members)), + [ + members[0].model_copy(update={"ingest_id": members[1].ingest_id}), + members[1].model_copy(update={"ingest_id": members[0].ingest_id}), + ], + [members[0].model_copy(update={"item_order": 2}), members[1]], + [ + members[0].model_copy( + update={ + "media_type": "application/vnd.openxmlformats-officedocument.wordprocessingml.document" + } + ), + members[1], + ], + ] + # Input order alone is irrelevant to normalized storage; canonical read restores order. + variants.pop(2) + for changed in variants: + async with h.factory() as session: + with pytest.raises(DBAPIError, match="canonical guide membership"): + await raw_insert(session, h, members=changed) + await session.commit() + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_deferred_failure_rolls_back(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + async with h.factory() as session: + await raw_insert(session, h, header={"packet_manifest_digest": "sha256:" + "0" * 64}) + with pytest.raises(DBAPIError, match="semantic digest mismatch"): + await session.commit() + await session.rollback() + async with h.factory() as session: + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) + == 0 + ) + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_guide_items")) + == 0 + ) + + +@pytest.mark.asyncio +async def test_packet_and_ingest_facts_are_immutable(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + async with h.factory() as session: + packet_id = await raw_insert(session, h) + await session.commit() + for table, column in ( + ("review_packet_manifests", "packet_manifest_digest"), + ("review_packet_guide_items", "media_type"), + ("guide_source_artifact_ingests", "media_type"), + ): + for statement in ( + f"UPDATE public.{table} SET {column}={column}", + f"DELETE FROM public.{table}", + f"TRUNCATE public.{table} CASCADE", + ): + async with h.factory() as session: + with pytest.raises(DBAPIError, match="immutable"): + await session.execute(text(statement)) + await session.rollback() + async with h.factory() as session: + assert ( + await session.scalar(text("SELECT id FROM public.review_packet_manifests")) + == packet_id + ) + assert await session.scalar( + text("SELECT count(*) FROM public.review_packet_guide_items") + ) == len(h.membership.guide_documents) + + +@pytest.mark.asyncio +async def test_packet_requires_active_exact_lease(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + async with h.factory() as session: + await session.execute( + text( + "UPDATE public.review_leases SET status='released',close_reason='manual_release',closed_at=clock_timestamp() WHERE id=:id" + ), + {"id": h.lease_id}, + ) + await session.execute( + text( + "UPDATE public.review_queue_entries SET queue_state='pending',active_lease_id=NULL WHERE id=:id" + ), + {"id": h.queue_id}, + ) + await session.commit() + async with h.factory() as session: + with pytest.raises(DBAPIError, match="requires exact active lease"): + await raw_insert(session, h) + await session.rollback() + + from app.core.identifiers import new_record_id + from app.modules.reviews.models import ReviewLease, ReviewQueueEntry + from app.modules.reviews.repository import ReviewQueueRepository + from app.modules.reviews.schemas import ReviewLeaseInput + + async with h.factory() as session: + prior = await session.get(ReviewLease, h.lease_id) + successor = await ReviewQueueRepository(session).add_lease( + ReviewLeaseInput( + id=new_record_id(), + review_queue_entry_id=h.queue_id, + project_id=prior.project_id, + task_id=prior.task_id, + submission_id=prior.submission_id, + submission_version=prior.submission_version, + reviewer_id=prior.reviewer_id, + reviewer_contribution_policy_version_id=prior.reviewer_contribution_policy_version_id, + attempt_generation=2, + expires_at=prior.expires_at, + ) + ) + queue = await session.get(ReviewQueueEntry, h.queue_id) + queue.queue_state = "leased" + successor_id = successor.id + queue.active_lease_id = successor_id + await session.commit() + with pytest.raises(DBAPIError, match="requires exact active lease"): + await raw_insert(session, h) + await session.rollback() + packet_id = await raw_insert( + session, + h, + header={"review_lease_id": successor_id, "packet_manifest_generation": 2}, + ) + await session.commit() + assert ( + await session.scalar( + text( + "SELECT packet_manifest_generation FROM public.review_packet_manifests WHERE id=:id" + ), + {"id": packet_id}, + ) + == 2 + ) + + +@pytest.mark.asyncio +async def test_packet_shadow_tables_cannot_change_custody(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + async with h.factory() as session: + for name in ( + "review_packet_manifests", + "review_packet_guide_items", + "submissions", + "checker_runs", + "guide_source_artifact_ingests", + "artifact_put_attempts", + "review_leases", + "review_queue_entries", + ): + await session.execute( + text(f"CREATE TEMP TABLE {name} (LIKE public.{name} INCLUDING ALL)") + ) + with pytest.raises(DBAPIError, match="canonical guide membership"): + await raw_insert(session, h, members=[]) + await session.commit() + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_rejects_numeric_version_and_generation(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + for field in ("submission_version", "packet_manifest_generation", "setup_generation"): + async with h.factory() as session: + with pytest.raises(DBAPIError): + await raw_insert(session, h, header={field: 999}) + await session.commit() + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_rejects_coherent_owner_substitutions(tmp_path, clean_postgres_database): + async with packet_source(tmp_path / "first", clean_postgres_database) as h: + async with packet_source( + tmp_path / "foreign", + clean_postgres_database, + storage_settings=h.settings, + provision_services=False, + ) as foreign: + await valid_control(h) + await valid_control(foreign) + foreign_header = foreign.membership.request.model_dump() + for field in ( + "project_id", + "task_id", + "submission_id", + "checker_run_id", + "result_id", + "guide_id", + "source_snapshot_id", + "project_setup_run_id", + ): + assert getattr(h.membership.request, field) != foreign_header[field] + async with h.factory() as session: + with pytest.raises(DBAPIError): + await raw_insert(session, h, header={field: foreign_header[field]}) + await session.commit() + await session.rollback() + for header, members in ( + ({"submission_binding_id": foreign.membership.submission.binding_id}, None), + ({}, foreign.membership.guide_documents), + ( + {}, + ( + *h.membership.guide_documents, + foreign.membership.guide_documents[0].model_copy(update={"item_order": 10}), + ), + ), + ): + async with h.factory() as session: + with pytest.raises(DBAPIError): + await raw_insert(session, h, header=header, members=members) + await session.commit() + await session.rollback() + # Stored foreign rows, not nonexistent UUIDs, exercise concealed reads. + from app.modules.reviews.packet.repository import ReviewPacketRepository + + async with h.factory() as session: + await raw_insert(session, foreign) + await session.commit() + assert ( + await ReviewPacketRepository(session).read( + h.membership.request.project_id, foreign.lease_id + ) + is None + ) + # A valid foreign member cannot be appended to an already committed packet. + async with h.factory() as session: + packet_id = await raw_insert(session, h) + await session.commit() + member = foreign.membership.guide_documents[0] + await session.execute( + text("INSERT INTO public.review_packet_guide_items VALUES (:p,:s,:i,10,:r,:m)"), + { + "p": packet_id, + "s": member.source_item_id, + "i": member.ingest_id, + "r": member.logical_role, + "m": member.media_type, + }, + ) + with pytest.raises(DBAPIError, match="canonical guide membership"): + await session.commit() + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_requires_committed_guide_upload(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + item = h.membership.guide_documents[0] + # Change only mutable put custody, leaving the prepared ingest and activation intact. + changes = ( + "receipt_id=NULL", + "status='prepared',terminal_result_code=NULL,terminal_at=NULL,receipt_id=NULL,replica_id=NULL,execution_generation=0,next_run_at=NULL,executor_id=NULL,lease_expires_at=NULL", + "request_digest='sha256:' || repeat('0',64)", + "byte_count=byte_count+1", + "sha256='sha256:' || repeat('0',64)", + "media_type='application/zip'", + ) + for assignment in changes: + async with h.factory() as session: + await session.execute( + text( + f"UPDATE public.artifact_put_attempts SET {assignment} WHERE guide_source_item_id=:id" + ), + {"id": item.source_item_id}, + ) + with pytest.raises(DBAPIError, match="canonical guide membership"): + await raw_insert(session, h) + await session.execute(text("SET CONSTRAINTS ALL IMMEDIATE")) + await session.rollback() + + async with h.factory() as session: + # A valid ZIP replica is still not this guide's original document. + await session.execute( + text( + "UPDATE public.artifact_put_attempts SET replica_id=:replica WHERE guide_source_item_id=:item" + ), + {"replica": h.material["replica_id"], "item": item.source_item_id}, + ) + with pytest.raises(DBAPIError, match="canonical guide membership"): + await raw_insert(session, h) + await session.execute(text("SET CONSTRAINTS ALL IMMEDIATE")) + await session.rollback() + # Namespace identity is already enforced earlier by ART's singleton FK. + with pytest.raises(DBAPIError, match="fk_artifact_put_attempts_namespace_fingerprint"): + await session.execute( + text( + "UPDATE public.artifact_put_attempts SET namespace_fingerprint='sha256:' || repeat('0',64) WHERE guide_source_item_id=:id" + ), + {"id": item.source_item_id}, + ) + await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_accepts_observed_confirmed_upload(tmp_path, clean_postgres_database): + from sqlalchemy import select + from app.core.identifiers import new_record_id + from app.modules.artifacts.models import ArtifactPutAttempt, ArtifactPutObservationReceipt + + async with packet_source(tmp_path, clean_postgres_database) as h: + async with h.factory() as session: + put = await session.scalar( + select(ArtifactPutAttempt).where( + ArtifactPutAttempt.guide_source_item_id + == str(h.membership.guide_documents[0].source_item_id) + ) + ) + observation = ArtifactPutObservationReceipt( + id=str(new_record_id()), + put_attempt_id=put.id, + execution_generation=put.execution_generation, + outcome="observed_confirmed", + expected_sha256=put.sha256, + expected_byte_count=put.byte_count, + observed_sha256=put.sha256, + observed_byte_count=put.byte_count, + ) + session.add(observation) + put.receipt_id = None + put.terminal_result_code = "document_stored_observed" + await session.commit() + await valid_control(h) + for assignment in ( + "execution_generation=execution_generation+1", + "byte_count=byte_count+1", + ): + async with h.factory() as session: + await session.execute( + text(f"UPDATE public.artifact_put_attempts SET {assignment} WHERE id=:id"), + {"id": put.id}, + ) + with pytest.raises(DBAPIError, match="canonical guide membership"): + await raw_insert(session, h) + await session.execute(text("SET CONSTRAINTS ALL IMMEDIATE")) + await session.rollback() + + +async def assert_packet_rejected(session, h, *, message, header=None, members=None): + with pytest.raises(DBAPIError, match=message): + await raw_insert(session, h, header=header, members=members) + await session.execute(text("SET CONSTRAINTS ALL IMMEDIATE")) + + +@pytest.mark.asyncio +async def test_packet_guard_removal_probes(tmp_path, clean_postgres_database): + """Negative assertions fail after removing their exact validator rejection.""" + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + for message, header, members in ( + ( + "review packet canonical guide membership mismatch", + None, + h.membership.guide_documents[:1], + ), + ( + "review packet semantic digest mismatch", + {"packet_manifest_digest": "sha256:" + "0" * 64}, + None, + ), + ): + async with h.factory() as session: + await assert_packet_rejected( + session, h, message=message, header=header, members=members + ) + await session.rollback() + definition = await session.scalar( + text( + "SELECT pg_get_functiondef('public.validate_review_packet(uuid)'::regprocedure)" + ) + ) + target = f"RAISE EXCEPTION '{message}' USING ERRCODE='23514';" + assert definition.count(target) == 1 + await session.execute(text(definition.replace(target, "NULL;"))) + with pytest.raises(pytest.fail.Exception, match="DID NOT RAISE"): + await assert_packet_rejected( + session, h, message=message, header=header, members=members + ) + await session.rollback() + async with h.factory() as session: + from app.core.identifiers import new_record_id + + with pytest.raises(DBAPIError, match="review packet parent unavailable"): + await session.execute( + text("SELECT public.validate_review_packet(:id)"), {"id": new_record_id()} + ) + await session.rollback() + for mode in ("receipt", "lease"): + message = ( + "review packet canonical guide membership mismatch" + if mode == "receipt" + else "review packet requires exact active lease" + ) + signature = ( + "public.validate_review_packet(uuid)" + if mode == "receipt" + else "public.guard_review_packet_creation()" + ) + + async def prepare(session): + if mode == "receipt": + await session.execute( + text( + "UPDATE public.artifact_put_attempts SET receipt_id=NULL WHERE guide_source_item_id=:id" + ), + {"id": h.membership.guide_documents[0].source_item_id}, + ) + else: + await session.execute( + text( + "UPDATE public.review_leases SET status='released',close_reason='manual_release',closed_at=clock_timestamp() WHERE id=:id" + ), + {"id": h.lease_id}, + ) + await session.execute( + text( + "UPDATE public.review_queue_entries SET queue_state='pending',active_lease_id=NULL WHERE id=:id" + ), + {"id": h.queue_id}, + ) + + async with h.factory() as session: + await prepare(session) + await assert_packet_rejected(session, h, message=message) + await session.rollback() + definition = await session.scalar( + text("SELECT pg_get_functiondef(CAST(:name AS regprocedure))"), + {"name": signature}, + ) + target = f"RAISE EXCEPTION '{message}' USING ERRCODE='23514';" + assert definition.count(target) == 1 + await session.execute(text(definition.replace(target, "NULL;"))) + await prepare(session) + with pytest.raises(pytest.fail.Exception, match="DID NOT RAISE"): + await assert_packet_rejected(session, h, message=message) + await session.rollback() + # Source-fact mutation has its own independently reached assertion. + async with h.factory() as session: + statement = text( + "UPDATE public.guide_source_artifact_ingests SET byte_count=byte_count+1 WHERE id=:id" + ) + params = {"id": h.membership.guide_documents[0].ingest_id} + with pytest.raises(DBAPIError, match="immutable"): + await session.execute(statement, params) + await session.rollback() + await session.execute( + text( + "DROP TRIGGER guide_source_artifact_ingests_immutable ON public.guide_source_artifact_ingests" + ) + ) + with pytest.raises(pytest.fail.Exception, match="DID NOT RAISE"): + with pytest.raises(DBAPIError, match="immutable"): + await session.execute(statement, params) + await session.rollback() diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index d5480eab9..24faed888 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -79,7 +79,7 @@ def test_v01_graph_has_one_root_and_head() -> None: script = ScriptDirectory.from_config(config) revisions = list(script.walk_revisions()) - assert [revision.revision for revision in revisions] == [HEAD_REVISION, "0010_post_submit_authority", "0009_checker_material_lineage", "0008_checker_execution", "0007_checker_output_custody", "0006_history_read_authority", "0005_task_evidence_authority", "0004_task_context_authority", "0003_task_read_authority", "0002_task_queue_authority", BASELINE_REVISION] + assert [revision.revision for revision in revisions] == [HEAD_REVISION, "0011_task_routing_source", "0010_post_submit_authority", "0009_checker_material_lineage", "0008_checker_execution", "0007_checker_output_custody", "0006_history_read_authority", "0005_task_evidence_authority", "0004_task_context_authority", "0003_task_read_authority", "0002_task_queue_authority", BASELINE_REVISION] assert revisions[-1].down_revision is None assert script.get_heads() == [HEAD_REVISION] diff --git a/backend/tests/test_behavior_ownership.py b/backend/tests/test_behavior_ownership.py index 34385eb34..b58b29bbd 100644 --- a/backend/tests/test_behavior_ownership.py +++ b/backend/tests/test_behavior_ownership.py @@ -2222,3 +2222,14 @@ def test_review_packet_contract_addition_preserves_existing_ownership() -> None: for targets in ([addition], [retained, addition, "backend/app/modules/artifacts/api/packet_reader.py"]): with pytest.raises(ownership.BehaviorOwnershipError, match="untrusted_partition_change"): ownership._validate_additive_partition_transition(_partition(sorted(targets)), trusted) + + +def test_review_packet_storage_addition_preserves_existing_ownership() -> None: + expected = {f"backend/app/modules/reviews/packet/{name}.py" for name in ("models", "schemas", "repository")} + assert ownership.REV_03B_PACKET_TARGETS == expected + retained = "backend/app/core/config.py" + trusted = _partition([retained]) + ownership._validate_additive_partition_transition(_partition(sorted({retained, *expected})), trusted) + for neighbor in ("backend/app/modules/reviews/packet/resolver.py", "backend/app/modules/reviews/claim.py"): + with pytest.raises(ownership.BehaviorOwnershipError, match="untrusted_partition_change"): + ownership._validate_additive_partition_transition(_partition(sorted({retained, *expected, neighbor})), trusted) diff --git a/backend/tests/test_ci_lane_catalogue.py b/backend/tests/test_ci_lane_catalogue.py index 9d3dd7780..6f80945a1 100644 --- a/backend/tests/test_ci_lane_catalogue.py +++ b/backend/tests/test_ci_lane_catalogue.py @@ -178,6 +178,9 @@ def test_measured_hotspots_have_explicit_semantic_owners() -> None: "tests/authorization/submission_history/test_migration.py", "tests/authorization/submission_history/test_absence.py", "tests/authorization/submission_history/test_failures.py", + "tests/reviews/packet/test_repository.py", + "tests/reviews/packet/test_storage.py", + "tests/reviews/packet/test_migration.py", "tests/tasks/post_submit_routing/test_contracts.py", "tests/tasks/post_submit_routing/test_storage.py", "tests/tasks/post_submit_routing/test_migration.py", diff --git a/backend/tests/test_identifier_schema.py b/backend/tests/test_identifier_schema.py index 995fa89c9..0865f7660 100644 --- a/backend/tests/test_identifier_schema.py +++ b/backend/tests/test_identifier_schema.py @@ -9,6 +9,7 @@ # These keys express business uniqueness, not generated record identities. NATURAL_PRIMARY_KEYS = { + "review_packet_guide_items": ("packet_id", "source_item_id"), "authority_control": ("id",), "checker_submission_fences": ("submission_id",), "artifact_storage_namespaces": ("id",), diff --git a/docs/architecture_data_model.md b/docs/architecture_data_model.md index a72052e62..631bf1581 100644 --- a/docs/architecture_data_model.md +++ b/docs/architecture_data_model.md @@ -1835,16 +1835,21 @@ REV's database guard rejects a draft, crossed-project, or lineage-mismatched identity. Reviewer and preferred-reviewer FKs accept only canonical human ActorProfiles. -`ReviewPacketManifest` remains planned REV persistence. The delivered -[ART-07A1 contract](../.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) -provides metadata-only types with distinct Submission and guide binding IDs, -not a packet table, resolver or byte capability. REV-03B will normalize those -members before complete REV-04A Review storage and shared FinalAcceptance. - -`ReviewPacketManifest` is an immutable REV semantic projection over the exact -lease, Submission, admitting CheckerRun/results, stamped context, response -evidence, and ART binding IDs. It contains no bytes, digest, provider locator, -signed URL, receipt, scratch path, or AUTH matrix data. +`ReviewPacketManifest` and `ReviewPacketGuideItem` provide immutable REV packet +storage. The header binds one lease/queue to the exact Submission/version, +admitting checker run/aggregate result, locked guide/snapshot and activated +setup run/generation. It contains exactly one original ZIP binding. Normalized +members identify each declared source item and live guide ingest, source order, +role and media type. PostgreSQL reconciles the complete set and committed uploads. + +The header's semantic `packet_manifest_digest` covers the full ART membership; +`packet_manifest_generation` equals the lease attempt generation. `created_at` +is database-owned. No artifact content hashes, sizes, provider locators, receipts, +source bodies or AUTH capabilities are copied. Header, members and ingests reject +mutation/deletion/truncation. Repository writes are caller-transaction operations; +exact replay retains the stored identity. This storage is delivered; the resolver, +claim authority and byte capability remain future work. Complete REV-04A Review +storage is next before shared FinalAcceptance. ## Review diff --git a/docs/engineering/authorization_activation_custody.md b/docs/engineering/authorization_activation_custody.md index 95b9a4b15..66da34f93 100644 --- a/docs/engineering/authorization_activation_custody.md +++ b/docs/engineering/authorization_activation_custody.md @@ -67,8 +67,8 @@ fixed-service AUTH/PREP and an exact current execution lease. Execute/finalize use the fixed `workstream.checker.post_submit` identity and phase-specific receipts. Do not implement an additional XINT-06B lane. [ART-07A1](../../.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) delivers -metadata-only packet types without authority or a resolver. REV-03B packet -storage and complete REV-04A Review storage precede shared FinalAcceptance; +metadata-only packet types without authority or a resolver. REV-03B normalized +packet storage is delivered; complete REV-04A Review storage is next before shared FinalAcceptance; hidden composition proof precedes exact activation. Runtime owner `WS-XINT-002-07` retains catalogue custody. The only approved diff --git a/docs/roadmap_status.md b/docs/roadmap_status.md index d56cf7278..3525e09fd 100644 --- a/docs/roadmap_status.md +++ b/docs/roadmap_status.md @@ -116,7 +116,7 @@ then branches on the locked ReviewPolicy: true routes to human `allow_review`; false invokes shared authorized acceptance without a human Review. Both routing integrations remain planned; false has scalar DTO proof only and guide activation still rejects it. ART-07A1 supplies metadata-only reviewer packet types, with no resolver or byte -authority. REV-03B packet persistence and complete REV-04A Review storage come +authority. REV-03B packet persistence is delivered; complete REV-04A Review storage comes next, before shared acceptance storage. Human review/revision, contribution and conditional compensation effects, operations and release proof complete v0.1. @@ -168,8 +168,8 @@ cannot be reused as post-submission review-gate evidence. See the | Contributor artifact preparation | **Hidden and proven** | One outer ZIP; bounded scratch inspection; canonical manifest; platform and project prechecks; unchanged-work rejection; durable put intent; verification; capacity-charged ready admission; hidden final handoff validates the exact activated historical guide through owner ports | Complete the later public admission-only cutover | | Pre-submission intake checking | **Hidden with approved-guide lineage** | Separate versioned pre-submission catalogue, locked effective-plan compilation, platform/project checks during continuous preparation, blocking feedback before Submission creation, and one internal phase command covering execution/replay with the JSON precheck removed; ARCH-03D connects approved-guide lineage through the final durable handoff | Complete the canonical public cutover after evaluation/remediation prerequisites; passing intake must never substitute for post-submit evaluation | | Immutable Submission creation | **Hidden foundation; public packet creation retired** | Contributor preparation authority; durable pre-submit reservation and exact completed-evidence recovery without rerunning checks; atomic admission consumption; TASK-owned admission-backed creation with exact assignment ContributionPolicyVersion and locked policy lineage; fixed-service artifact binding; replay/concurrency/rollback proof | Finish downstream evaluation and the canonical public integration. The retained submission-list GET is not a usable creation POST | -| Post-submission evaluation and `allow_review` | **Hidden source foundation; routing planned** | One canonical CHECKER post-submit catalogue/compiler used by existing consumers, internal phase service with exact fixed-service post-submit authority, hidden value contracts and structural-handler conformance; ARCH-04B/04B2 input and output custody; ARCH-04C durable execution and current-result custody; ARCH-04D1 canonical ART material custody; ARCH-04D2 exact phase authority and receipts; ARCH-04E1A immutable route-neutral source table, detached facts and type-only accepted-effects Protocol | Build REV-03B packet and REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence foundations, then 04E1B/04E2/04E3 dispatch and routing plus 04F remediation. Before publication, harden the same source table with mandatory exact route/owner receipts and refuse retained pre-authority rows. No writer, reader, handler, current pointer, route or acceptance effect is live | -| Review queue and lease | **Hidden persistence foundation** | Queue/admission idempotency and ReviewLease/preference persistence; complete unavailable REV action/principal catalogue and typed AUTH contracts; ART-07A1 metadata-only packet contract | REV-03B normalized packet persistence and resolver proof; REV-04A Review schema; canonical admission from `allow_review`; claim/lease/packet authority; lease copies the Submission-stamped policy version with no CON lookup | +| Post-submission evaluation and `allow_review` | **Hidden source foundation; routing planned** | One canonical CHECKER post-submit catalogue/compiler used by existing consumers, internal phase service with exact fixed-service post-submit authority, hidden value contracts and structural-handler conformance; ARCH-04B/04B2 input and output custody; ARCH-04C durable execution and current-result custody; ARCH-04D1 canonical ART material custody; ARCH-04D2 exact phase authority and receipts; ARCH-04E1A immutable route-neutral source table, detached facts and type-only accepted-effects Protocol | Build complete REV-04A Review storage, then shared REV-04B/CON-03C/07 and REV-12A/CON fence foundations, then 04E1B/04E2/04E3 dispatch and routing plus 04F remediation. Before publication, harden the same source table with mandatory exact route/owner receipts and refuse retained pre-authority rows. No writer, reader, handler, current pointer, route or acceptance effect is live | +| Review queue and lease | **Hidden persistence foundation** | Queue/admission idempotency and ReviewLease/preference persistence; complete unavailable REV action/principal catalogue and typed AUTH contracts; ART-07A1 metadata-only packet contract and REV-03B immutable normalized packet persistence with live ingest custody | Complete REV-04A Review schema; future resolver proof; canonical admission from `allow_review`; claim/lease/packet authority; lease copies the Submission-stamped policy version with no CON lookup | | Review decision and revision | **Planned** | Review/revision policy identities and mutation authority; approved same-task revision-rebase semantics | Immutable findings and decisions; `accept`, `needs_revision`, and `reject`; complete-context revision preparation; finding responses; replacement contributor rules; replay and recovery | | Contribution and compensation truth | **Schema foundations plus public policy administration** | ContributionPolicyVersion persistence; lifecycle-audit participant; adapter bindings; public Finance policy administration | Persist ContributionRecord/CompensationAward and one shared FinalAcceptance/submitter operation for human accept or authorized false/pass routing. Only actual Reviews create reviewer records. Evaluate frozen actor rules into zero, one or two awards | | Fulfillment, reconciliation, and audit | **Planned** | Shared audit foundations, provider-neutral adapter convention, AUTH-OUTBOX-02 live dispatcher authority, retained phase audit decisions, Celery delivery/recovery scans and CON-02B custody | Feature-specific handlers and authority, conditional award fulfillment, callbacks, idempotent recovery, reconciliation, bounded operational reads, and release controls | @@ -466,7 +466,7 @@ The next dependency-safe product sequence is: ARCH-04D2 supplies exact input/execute/finalize service authority and durable receipt custody. ARCH-04E1A supplies one immutable route-neutral source table, detached facts and source-neutral accepted-effects types. It has no runtime - entry. REV-03B packet and REV-04A Review storage, then shared REV-04B/CON-03C/07 and the existing REV-12A/CON fence foundation + entry. Complete REV-04A Review storage, then shared REV-04B/CON-03C/07 and the existing REV-12A/CON fence foundation come next; ARCH-04E1B/04E2/04E3 then dispatch evaluation and publish an exact human `allow_review` manifest on true when no blocking failure exists. CHECKERS owns durable execution/currentness; the shared facade does not @@ -477,7 +477,7 @@ The next dependency-safe product sequence is: approved catalogue-bound generation. Infrastructure retries and project setup faults are not contributor failures; `allow_review` is not acceptance. **For the first false-policy acceptance path:** consume the delivered TASK - 04E1A source facts in REV-04B's source FK after REV-03B packet and complete + 04E1A source facts in REV-04B's source FK after delivered REV-03B packet storage and complete REV-04A Review storage; complete CON-03C/07 and the existing shared fence/controller slice, then wire one shared acceptance operation through 04E1B/04E2/04E3. Prove real scoped activation/drain and 04F remediation before @@ -583,10 +583,11 @@ Delivered foundations (not a claim of full public integration) ARCH-04D2 exact input/execute/finalize authority + durable receipts ARCH-04E1A immutable route-neutral TASK source facts + type-only effects port ART-07A1 metadata-only packet membership types (no resolver/byte authority) + REV-03B normalized immutable lease packets and live guide ingest custody | v Remaining integration - REV-03B normalized packet -> REV-04A complete Review storage + REV-04A complete Review storage -> shared REV-04B/CON-03C/07 + REV-12A/CON fence foundation -> shared acceptance composition -> 04E1B/04E2/04E3 dispatch/routing -> 04F remediation @@ -709,7 +710,8 @@ remaining trace sequence is: lead to delivered [ARCH-04E1A source facts](../.commitrail/initiatives/WS-ARCH-001/WS-ARCH-001-04E1A.md). Delivered [ART-07A1 packet types](../.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) - precede REV-03B packet and REV-04A Review storage, then shared + and [REV-03B packet persistence](../.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md) + precede complete REV-04A Review storage, then shared REV-04B/CON-03C/07 and the REV-12A/CON fence foundation before `04E1B -> 04E2 -> 04E3` dispatch and routing; 04F supplies remediation. The mandatory [04D1 canonical material custody](../.commitrail/initiatives/WS-ARCH-001/WS-ARCH-001-04D1.md) diff --git a/docs/spec_artifact_storage_service.md b/docs/spec_artifact_storage_service.md index b4dd0639d..63efdd2f0 100644 --- a/docs/spec_artifact_storage_service.md +++ b/docs/spec_artifact_storage_service.md @@ -1624,8 +1624,9 @@ prevents a live contributor route whose mandatory checker read is unavailable. [ART-07A1](../.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) supplies metadata-only packet membership types and a type-only port. It implements no -resolver, guide-binding writer or byte access. REV-03B normalized packet storage -and complete REV-04A Review storage precede shared FinalAcceptance. +resolver or byte access. REV-03B now stores normalized packets referencing live +guide ingests with committed-upload custody; retained extraction bindings remain +read-only. Complete REV-04A Review storage is next before shared FinalAcceptance. ART later supplies an exact, authorized reviewer-packet byte capability, while REV owns queueing, leases, decisions, and the reviewer note/findings. The approved v0.1 review flow does not upload a reviewer revision artifact. CON owns diff --git a/docs/spec_review_lifecycle.md b/docs/spec_review_lifecycle.md index 467c9dba9..acc4276f4 100644 --- a/docs/spec_review_lifecycle.md +++ b/docs/spec_review_lifecycle.md @@ -327,12 +327,13 @@ keys, provider URIs, scratch paths, receipts, or credentials. `ReviewPacketManifest` is an immutable REV semantic projection naming the exact queue entry, lease, versioned Submission, admitting CheckerRun/results, stamped -guide or revision context, bounded response relations, and ART binding IDs. It -stores no bytes, content digest, provider location, signed URL, scratch path, +guide/setup context, original ZIP binding and live guide ingest IDs. Revision +response relations belong to the later complete Review storage boundary. It +stores no bytes, artifact content digest, provider location, signed URL, scratch path, receipt, or authorization-matrix data. -An active ReviewLease authorizes artifact bytes only for the single Submission -packet named by its manifest. Prior, expired, consumed, sibling, later, +Future exact lease/packet authorization permits artifact bytes only for the single +Submission packet named by its manifest; an active stored lease alone grants none. Prior, expired, consumed, sibling, later, cross-task, and cross-project leases cannot read those bytes. Authorized chain history may expose bounded binding ID, relation, media type, verification/availability, and required/optional metadata, but never bytes, @@ -346,19 +347,22 @@ Project Manager/Operator. Prior participation grants metadata history only; artifact bytes still require the current active lease for the exact packet. -The delivered [ART-07A1 membership contract](../.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) -provides strict detached types and a type-only async port, not a resolver. Its -scope includes exact Submission/version, aggregate CheckerRun result identity, -locked guide/snapshot and activated setup run/generation. It names one required -original ZIP binding and 1–100 ordered original guide bindings using their -separate ART owners. Both member kinds are always required. The guide binding -table exists, but its packet writer/resolver is not implemented. Shape and echoed -request validation prove neither stored ownership nor complete membership; -future canonical-owner reads must establish both within the caller transaction. -REV-03B must retain normalized members; no opaque JSON binding set. Byte access -still requires separate exact lease/packet authorization. The current catalogue -has no output files; adding those later requires an explicit owner-custody -contract, not arbitrary additional packet members. +The [ART-07A1 membership contract](../.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) +defines strict detached types and a type-only async port. REV-03B persists one +immutable metadata packet per exact active ReviewLease: the original Submission +ZIP binding and all 1–100 ordered guide documents from the locked activated setup. +Guide members name live `ingest_id` values, not the retained extraction binding +table. Canonical PostgreSQL checks reconcile every owner and the complete declared +set, including committed upload receipts. A semantic digest covers the complete +membership; packet generation equals the lease attempt generation. Packet and +ingest facts reject mutation and deletion; identical replay returns the same ID, +including after lease closure or guide supersession. The caller owns the transaction. + +This delivers storage, not claim, a membership resolver or byte access. Future +canonical-owner reads must construct the complete membership, and byte access +requires separate exact lease/packet authorization and current availability checks. +The current catalogue has no output files; adding those requires an explicit +owner-custody contract, not arbitrary additional packet members. ## Review Notes, Findings, And Revision Responses @@ -572,8 +576,8 @@ Extract foundations from existing owner work, not a new initiative: not restricted to human admission. False proof is scalar transport only while activation remains unavailable. This schema precedes the REV source FK. 2. [ART-07A1](../.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md) supplies - the metadata-only packet contract. Next implement REV-03B normalized packet - persistence, then complete REV-04A Review-source storage before REV-04B + the metadata-only packet contract. REV-03B normalized packet persistence is + delivered. Next complete REV-04A Review-source storage before REV-04B shared FinalAcceptance storage. Do not create an incomplete Review solely as an FK target. CON-03C contribution/award persistence and CON-07 submitter participation follow. These storage prerequisites require no live human From e533c5263b84e9273cd3a327d36b61c225e09bcd Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 03:39:04 +0100 Subject: [PATCH 5/9] docs(rev): keep packet owner correction in current change record --- .commitrail/initiatives/WS-ART-001/OVERVIEW.md | 4 +++- .commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md | 8 -------- .commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md | 5 +++-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/.commitrail/initiatives/WS-ART-001/OVERVIEW.md b/.commitrail/initiatives/WS-ART-001/OVERVIEW.md index 5f4cb4bcd..5a33d10e6 100644 --- a/.commitrail/initiatives/WS-ART-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-ART-001/OVERVIEW.md @@ -18,7 +18,9 @@ and the [capability ledger](../../../docs/roadmap_status.md). writer or published route. Output-file authority remains unavailable for the zero-output catalogue. - Delivered contract: [ART-07A1](WS-ART-001-07A1.md) defines exact, metadata-only - reviewer packet membership. It supplies no resolver or byte authority. + reviewer packet membership. REV-03B corrects its original `guide_binding_id` + to live upload `ingest_id`; the completed ART record preserves the original + delivery history. No alias, revived extraction writer or byte authority. - Delivered storage: [REV-03B](../WS-REV-001/WS-REV-001-03B.md) freezes exact lease packets with normalized live guide ingests; no resolver or byte authority. - Next usable boundary: complete REV-04A diff --git a/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md b/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md index cf094ccfa..d1316bced 100644 --- a/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md +++ b/.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md @@ -193,11 +193,3 @@ Implementation review tightened native Python UUID validation (JSON decoding remains supported) and reconciled later human-runtime steps with storage already required by automated acceptance. The substitution regression rejects valid UUID strings at request and both member boundaries; no compatibility coercion remains. - -## Current owner correction - -[REV-03B](../WS-REV-001/WS-REV-001-03B.md) replaces the originally delivered -`guide_binding_id` with `ingest_id` from live guide uploads. The former target is -retained extraction evidence whose writes are sealed. No alias, revived writer -or data deletion is introduced. The original delivery record above is historical; -current packet consumers use only the corrected contract. diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index 904130f72..ccd2698ec 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -19,7 +19,9 @@ upload and guide manifest use `GuideSourceArtifactIngest.id`, exposed as `ingest_id` by the PROJECTS public guide document contract. Replace the mistaken field with that live identity in this affected scope; do not activate the old table, add an alias, or delete retained data. Existing extraction evidence stays -read-only. The complete packet remains metadata-only. +read-only. The complete packet remains metadata-only. The completed ART-07A1 +record remains unchanged as delivery history; this record and the ART overview +carry the owner correction, preserving one change record per implementation PR. ## Bounded change @@ -49,7 +51,6 @@ read-only. The complete packet remains metadata-only. ### Allowed current documentation - This record; `.commitrail/INDEX.md`. -- `.commitrail/initiatives/WS-ART-001/WS-ART-001-07A1.md`: document the current corrected guide identity and link this repair, preserving its original delivery history. - `.commitrail/initiatives/WS-ART-001/OVERVIEW.md` - `.commitrail/initiatives/WS-REV-001/OVERVIEW.md` - `.commitrail/initiatives/WS-ARCH-001/OVERVIEW.md` From 58001b7a23fad8164eb333b5f86a51fbfdc28905 Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 03:54:50 +0100 Subject: [PATCH 6/9] test(rev): prove packet lease receipt and historical guide custody --- .../initiatives/WS-REV-001/WS-REV-001-03B.md | 11 +- backend/tests/reviews/packet/support.py | 177 +++++++++--------- .../tests/reviews/packet/test_repository.py | 9 +- backend/tests/reviews/packet/test_storage.py | 95 ++++++++++ 4 files changed, 201 insertions(+), 91 deletions(-) diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index ccd2698ec..bd4182f59 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -246,7 +246,13 @@ These tests bind the storage boundary; execution results and review freshness be project, task, Submission/version, run/result, activated setup/generation, ZIP. - `test_packet_rejects_null_source_fields`: each nonnullable source independently. - `test_packet_creation_time_is_database_owned`: supplied NULL/past/future times. -- `test_packet_requires_active_exact_lease`: terminal and sibling lease insertions. +- `test_packet_requires_active_exact_lease`: terminal and successor lease insertions. +- `test_packet_rejects_coherent_sibling_under_another_lease`: a complete valid + same-project sibling packet cannot use another lease; removing only the + creation tuple comparison makes the rejection assertion fail. +- `test_packet_rejects_another_documents_valid_receipt`: a valid receipt from + another document cannot attest this upload; removing the receipt linkage + predicates makes the rejection assertion fail. - `test_packet_requires_committed_guide_upload`: prepared-only ingest cannot pass; independently crossed content, replica, namespace, receipt, ingest byte and media identities fail after a valid operation-receipt control. Namespace @@ -263,7 +269,8 @@ These tests bind the storage boundary; execution results and review freshness be - `test_packet_creation_serializes_with_lease_closure`: cannot create post-close. - `test_packet_read_conceals_foreign_project`: real stored foreign identity. - `test_packet_retains_superseded_guide_lineage`: real successor activation, then - historical read and identical replay without selecting the successor. + first packet creation, historical read and identical replay without selecting + the successor. An active-only validator must reject that new creation. - `test_packet_shadow_tables_cannot_change_custody`: temporary name shadowing. - `test_packet_upgrade_preserves_existing_owners`: populated predecessor upgrade. diff --git a/backend/tests/reviews/packet/support.py b/backend/tests/reviews/packet/support.py index e3e8544fc..6fe358681 100644 --- a/backend/tests/reviews/packet/support.py +++ b/backend/tests/reviews/packet/support.py @@ -34,100 +34,103 @@ @asynccontextmanager async def packet_source(tmp_path, database_url, **source_options): async with completed_source(tmp_path, database_url, **source_options) as h: - async with h.factory() as session: - submission = await session.get(Submission, str(h.request.submission_id)) - guide = await session.scalar( - select(ProjectGuide).where( - ProjectGuide.project_id == str(h.source["project_id"]), - ProjectGuide.version == submission.locked_guide_version, - ) + await prepare_packet(h) + yield h + + +async def prepare_packet(h): + """Attach canonical packet membership and a committed lease to a completed source.""" + async with h.factory() as session: + submission = await session.get(Submission, str(h.request.submission_id)) + guide = await session.scalar( + select(ProjectGuide).where( + ProjectGuide.project_id == str(h.source["project_id"]), + ProjectGuide.version == submission.locked_guide_version, ) - activation = await session.scalar( - select(GuideMutationIdempotencyRecord).where( - GuideMutationIdempotencyRecord.operation_id == guide.activation_operation_id, - ) + ) + activation = await session.scalar( + select(GuideMutationIdempotencyRecord).where( + GuideMutationIdempotencyRecord.operation_id == guide.activation_operation_id, ) - proposal = activation.response_json["command"]["target"]["proposal"] - rows = ( - await session.execute( - select(GuideSourceSnapshotItem, GuideSourceArtifactIngest) - .join( - GuideSourceArtifactIngest, - GuideSourceArtifactIngest.source_item_id == GuideSourceSnapshotItem.id, - ) - .where( - GuideSourceSnapshotItem.source_snapshot_id - == submission.locked_guide_source_snapshot_id - ) - .order_by(GuideSourceSnapshotItem.item_order) + ) + proposal = activation.response_json["command"]["target"]["proposal"] + rows = ( + await session.execute( + select(GuideSourceSnapshotItem, GuideSourceArtifactIngest) + .join( + GuideSourceArtifactIngest, + GuideSourceArtifactIngest.source_item_id == GuideSourceSnapshotItem.id, ) - ).all() - h.membership = ReviewPacketMembership( - request=ReviewPacketMembershipRequest( - project_id=h.source["project_id"], - task_id=h.request.task_id, - submission_id=h.request.submission_id, - submission_version=submission.version, - checker_run_id=h.result.attempt_id, - result_id=h.source["result_id"], - guide_id=UUID(guide.id), - guide_version=guide.version, - source_snapshot_id=UUID(submission.locked_guide_source_snapshot_id), - project_setup_run_id=UUID(proposal["setup_run_id"]), - setup_generation=proposal["setup_generation"], - ), - submission=ReviewSubmissionMember( - binding_id=UUID(submission.artifact_binding_id), - logical_role="submission_bundle_original", - media_type="application/zip", - ), - guide_documents=tuple( - ReviewGuideMember( - ingest_id=UUID(ingest.id), - source_item_id=UUID(item.id), - item_order=item.item_order, - logical_role="guide_source_original", - media_type=item.media_type, - ) - for item, ingest in rows - ), - ) - repository = ReviewQueueRepository(session) - queue = await repository.add_queue_entry( - ReviewQueueEntryInput( - id=new_record_id(), - project_id=str(h.source["project_id"]), - task_id=str(h.request.task_id), - submission_id=submission.id, - submission_version=submission.version, - admitting_checker_run_id=str(h.result.attempt_id), - routing_mode=ReviewRoutingMode.OPEN, - routing_reason=ReviewRoutingReason.FIRST_SUBMISSION, + .where( + GuideSourceSnapshotItem.source_snapshot_id + == submission.locked_guide_source_snapshot_id ) + .order_by(GuideSourceSnapshotItem.item_order) ) - await session.commit() - actor_id = await _human_actor(session, label="packet-reviewer") - lease = await repository.add_lease( - ReviewLeaseInput( - id=new_record_id(), - review_queue_entry_id=queue.id, - project_id=queue.project_id, - task_id=queue.task_id, - submission_id=submission.id, - submission_version=submission.version, - reviewer_id=actor_id, - reviewer_contribution_policy_version_id=h.source[ - "contribution_policy_version_id" - ], - attempt_generation=1, - expires_at=datetime.now(UTC) + timedelta(days=1), + ).all() + h.membership = ReviewPacketMembership( + request=ReviewPacketMembershipRequest( + project_id=h.source["project_id"], + task_id=h.request.task_id, + submission_id=h.request.submission_id, + submission_version=submission.version, + checker_run_id=h.result.attempt_id, + result_id=h.source["result_id"], + guide_id=UUID(guide.id), + guide_version=guide.version, + source_snapshot_id=UUID(submission.locked_guide_source_snapshot_id), + project_setup_run_id=UUID(proposal["setup_run_id"]), + setup_generation=proposal["setup_generation"], + ), + submission=ReviewSubmissionMember( + binding_id=UUID(submission.artifact_binding_id), + logical_role="submission_bundle_original", + media_type="application/zip", + ), + guide_documents=tuple( + ReviewGuideMember( + ingest_id=UUID(ingest.id), + source_item_id=UUID(item.id), + item_order=item.item_order, + logical_role="guide_source_original", + media_type=item.media_type, ) + for item, ingest in rows + ), + ) + repository = ReviewQueueRepository(session) + queue = await repository.add_queue_entry( + ReviewQueueEntryInput( + id=new_record_id(), + project_id=str(h.source["project_id"]), + task_id=str(h.request.task_id), + submission_id=submission.id, + submission_version=submission.version, + admitting_checker_run_id=str(h.result.attempt_id), + routing_mode=ReviewRoutingMode.OPEN, + routing_reason=ReviewRoutingReason.FIRST_SUBMISSION, ) - queue.queue_state = "leased" - queue.active_lease_id = lease.id - await session.commit() - h.lease_id, h.queue_id = lease.id, queue.id - yield h + ) + await session.commit() + actor_id = await _human_actor(session, label="packet-reviewer") + lease = await repository.add_lease( + ReviewLeaseInput( + id=new_record_id(), + review_queue_entry_id=queue.id, + project_id=queue.project_id, + task_id=queue.task_id, + submission_id=submission.id, + submission_version=submission.version, + reviewer_id=actor_id, + reviewer_contribution_policy_version_id=h.source["contribution_policy_version_id"], + attempt_generation=1, + expires_at=datetime.now(UTC) + timedelta(days=1), + ) + ) + queue.queue_state = "leased" + queue.active_lease_id = lease.id + await session.commit() + h.lease_id, h.queue_id = lease.id, queue.id async def raw_insert(session, h, *, header=None, members=None): diff --git a/backend/tests/reviews/packet/test_repository.py b/backend/tests/reviews/packet/test_repository.py index 4a0b780d1..eda3c63c1 100644 --- a/backend/tests/reviews/packet/test_repository.py +++ b/backend/tests/reviews/packet/test_repository.py @@ -213,8 +213,6 @@ async def test_packet_retains_superseded_guide_lineage(tmp_path, clean_postgres_ async with packet_source(tmp_path, clean_postgres_database) as h: async with h.factory() as session: - original = await ReviewPacketRepository(session).store(h.lease_id, h.membership) - await session.commit() guide = await session.get(ProjectGuide, str(h.membership.request.guide_id)) first = await load_guide_activation(session, guide) grant, link = ( @@ -251,6 +249,13 @@ async def test_packet_retains_superseded_guide_lineage(tmp_path, clean_postgres_ async with h.factory() as session: guide = await session.get(ProjectGuide, str(h.membership.request.guide_id)) assert guide.status == "superseded" + # First creation must resolve the historical guide, not only replay a packet. + original = await ReviewPacketRepository(session).store(h.lease_id, h.membership) + await session.commit() + assert original.membership == h.membership + assert original.packet_manifest_digest == canonical_json_hash( + h.membership.model_dump(mode="json") + ) assert ( await ReviewPacketRepository(session).read( h.membership.request.project_id, h.lease_id diff --git a/backend/tests/reviews/packet/test_storage.py b/backend/tests/reviews/packet/test_storage.py index f2d858739..4e98595d8 100644 --- a/backend/tests/reviews/packet/test_storage.py +++ b/backend/tests/reviews/packet/test_storage.py @@ -530,3 +530,98 @@ async def prepare(session): with pytest.raises(DBAPIError, match="immutable"): await session.execute(statement, params) await session.rollback() + + +@pytest.mark.asyncio +async def test_packet_rejects_coherent_sibling_under_another_lease( + tmp_path, clean_postgres_database +): + from tests.reviews.packet.support import prepare_packet + from tests.tasks.post_submit_routing.support import completed_sibling_source + + async with packet_source(tmp_path, clean_postgres_database) as h: + sibling = await completed_sibling_source(h) + await prepare_packet(sibling) + await valid_control(h) + await valid_control(sibling) + assert h.membership.request.project_id == sibling.membership.request.project_id + assert h.membership.request.task_id != sibling.membership.request.task_id + assert h.membership.request.submission_id != sibling.membership.request.submission_id + assert h.membership.request.checker_run_id != sibling.membership.request.checker_run_id + assert h.queue_id != sibling.queue_id + assert h.lease_id != sibling.lease_id + # Every header/member/digest is coherent for the sibling. Only the lease is foreign. + header = {"review_lease_id": h.lease_id} + message = "review packet requires exact active lease" + async with h.factory() as session: + await assert_packet_rejected(session, sibling, message=message, header=header) + await session.rollback() + definition = await session.scalar( + text( + "SELECT pg_get_functiondef('public.guard_review_packet_creation()'::regprocedure)" + ) + ) + start = definition.index("OR (NEW.review_queue_entry_id,NEW.task_id") + end = definition.index("\n THEN", start) + # Omit only the tuple equality; active lease/queue and all FKs remain enforced. + await session.execute(text(definition[:start] + definition[end:])) + with pytest.raises(pytest.fail.Exception, match="DID NOT RAISE"): + await assert_packet_rejected(session, sibling, message=message, header=header) + await session.rollback() + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) + == 0 + ) + + +@pytest.mark.asyncio +async def test_packet_rejects_another_documents_valid_receipt(tmp_path, clean_postgres_database): + async with packet_source(tmp_path, clean_postgres_database) as h: + await valid_control(h) + first, second = h.membership.guide_documents + async with h.factory() as session: + receipt = ( + await session.execute( + text( + "SELECT receipt.id,receipt.guide_source_item_id,receipt.put_attempt_id,receipt.replica_id " + "FROM public.artifact_put_attempts put " + "JOIN public.artifact_operation_receipts receipt ON receipt.id=put.receipt_id " + "WHERE put.guide_source_item_id=:item AND put.status='object_confirmed' " + "AND receipt.outcome='document_stored'" + ), + {"item": second.source_item_id}, + ) + ).one() + assert receipt.guide_source_item_id == second.source_item_id + assert receipt.guide_source_item_id != first.source_item_id + await session.rollback() + + async def substitute(): + await session.execute( + text( + "UPDATE public.artifact_put_attempts SET receipt_id=:receipt WHERE guide_source_item_id=:item" + ), + {"receipt": receipt.id, "item": first.source_item_id}, + ) + + message = "review packet canonical guide membership mismatch" + await substitute() + await assert_packet_rejected(session, h, message=message) + await session.rollback() + definition = await session.scalar( + text( + "SELECT pg_get_functiondef('public.validate_review_packet(uuid)'::regprocedure)" + ) + ) + start = definition.index(" AND receipt.put_attempt_id=put.id") + end = definition.index("AND receipt.outcome='document_stored'", start) + # Keep the valid receipt ID/outcome and every other packet/upload guard. + await session.execute(text(definition[:start] + " " + definition[end:])) + await substitute() + with pytest.raises(pytest.fail.Exception, match="DID NOT RAISE"): + await assert_packet_rejected(session, h, message=message) + await session.rollback() + assert ( + await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) + == 0 + ) From a7da8f15c1fc7fe2257239de06c1cb9dba771320 Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 04:15:33 +0100 Subject: [PATCH 7/9] test(checkers): follow canonical migration head after packet storage --- .commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md | 2 ++ backend/tests/checkers/execution/test_migration.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index bd4182f59..e9f22d7ef 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -44,6 +44,8 @@ carry the owner correction, preserving one change record per implementation PR. - `backend/tests/reviews/packet/test_migration.py` - `backend/tests/conftest.py`: exact new resettable/guarded tables and measured schema fingerprint; no weaker validation. - `backend/tests/test_alembic.py`: exact revision graph. +- `backend/tests/checkers/execution/test_migration.py`: use the existing shared + current-head fixture for fresh migration; retain all execution-custody assertions. - `backend/scripts/identifier_inventory.py` and `backend/tests/test_identifier_schema.py`: classify only the new natural packet/source-item primary key; preserve all UUID guards. - `backend/scripts/test_lane_catalogue.py` and `backend/tests/test_ci_lane_catalogue.py`: additive registration in existing task lanes. - `backend/scripts/behavior_ownership.py`, `backend/tests/test_behavior_ownership.py`, `.ci/behavior-ownership/partition.v1.json`: exact three-module REV addition and neighbor-rejection proof. diff --git a/backend/tests/checkers/execution/test_migration.py b/backend/tests/checkers/execution/test_migration.py index ffbd4c530..7a5e0df90 100644 --- a/backend/tests/checkers/execution/test_migration.py +++ b/backend/tests/checkers/execution/test_migration.py @@ -9,7 +9,7 @@ from sqlalchemy.exc import IntegrityError from app.core.identifiers import new_record_id -from tests.migration_fixtures import _config +from tests.migration_fixtures import _config, current_schema_revision from tests.post_submit_materialization_helpers import material_fixture pytestmark = pytest.mark.postgres_schema_contract @@ -144,7 +144,7 @@ async def test_empty_database_installs_execution_custody(isolated_database_env, try: assert ( await conn.fetchval("select version_num from alembic_version") - == "0011_task_routing_source" + == current_schema_revision() ) assert await conn.fetchval("select count(*) from checker_submission_fences") == 0 columns = set( From 71ab3444a4900fb07b0c85fa7a0d5c6b2e11187d Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 04:25:44 +0100 Subject: [PATCH 8/9] test(art): preserve truncate guard proof with packet foreign key --- .../initiatives/WS-REV-001/WS-REV-001-03B.md | 2 ++ backend/tests/test_checker_output_storage.py | 22 ++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index e9f22d7ef..0a052915a 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -44,6 +44,8 @@ carry the owner correction, preserving one change record per implementation PR. - `backend/tests/reviews/packet/test_migration.py` - `backend/tests/conftest.py`: exact new resettable/guarded tables and measured schema fingerprint; no weaker validation. - `backend/tests/test_alembic.py`: exact revision graph. +- `backend/tests/test_checker_output_storage.py`: retain artifact-binding TRUNCATE + rejection and guard-removal proof after adding the packet foreign key. - `backend/tests/checkers/execution/test_migration.py`: use the existing shared current-head fixture for fresh migration; retain all execution-custody assertions. - `backend/scripts/identifier_inventory.py` and `backend/tests/test_identifier_schema.py`: classify only the new natural packet/source-item primary key; preserve all UUID guards. diff --git a/backend/tests/test_checker_output_storage.py b/backend/tests/test_checker_output_storage.py index fb5e3f2cf..c570f4653 100644 --- a/backend/tests/test_checker_output_storage.py +++ b/backend/tests/test_checker_output_storage.py @@ -753,7 +753,7 @@ async def test_artifact_binding_truncate_custody_blocks_direct_and_cascade_delet ) for statement in ( - "truncate artifact_bindings", + "truncate artifact_bindings cascade", "truncate artifact_contents cascade", ): with pytest.raises(DBAPIError, match="artifact_bindings rows are immutable"): @@ -768,6 +768,16 @@ async def test_artifact_binding_truncate_custody_blocks_direct_and_cascade_delet async with case.factory() as session: transaction = await session.begin() try: + # Isolate the binding trigger from the new inbound packet FK. This + # rollback-only probe has no packets; production custody stays intact. + assert await session.scalar(text("select count(*) from review_packet_manifests")) == 0 + await session.execute(text( + "alter table review_packet_manifests drop constraint " + "fk_review_packet_manifests_submission_binding_id_artifa_4ee3" + )) + with pytest.raises(DBAPIError, match="artifact_bindings rows are immutable"): + async with session.begin_nested(): + await session.execute(text("truncate artifact_bindings")) await session.execute( text( "alter table artifact_bindings disable trigger " @@ -797,6 +807,16 @@ async def test_artifact_binding_truncate_custody_blocks_direct_and_cascade_delet assert trigger_definition is not None assert "BEFORE TRUNCATE" in trigger_definition assert "EXECUTE FUNCTION reject_artifact_fact_mutation()" in trigger_definition + assert await session.scalar(text( + "select tgenabled='O' from pg_trigger " + "where tgrelid='artifact_bindings'::regclass " + "and tgname='trg_artifact_bindings_no_truncate'" + )) is True + assert await session.scalar(text( + "select count(*) from pg_constraint " + "where conrelid='review_packet_manifests'::regclass " + "and conname='fk_review_packet_manifests_submission_binding_id_artifa_4ee3'" + )) == 1 @pytest.mark.asyncio From 49be77efe361823b08fe380fd97639d3ae634e5a Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 2 Oct 2026 08:55:18 +0100 Subject: [PATCH 9/9] fix(rev): enforce packet lease deadline and strengthen custody proofs --- .../initiatives/WS-REV-001/WS-REV-001-03B.md | 38 +++- .../alembic/versions/0012_review_packet.py | 1 + .../app/modules/reviews/packet/repository.py | 8 +- backend/app/modules/reviews/packet/schemas.py | 2 +- backend/tests/conftest.py | 2 +- backend/tests/reviews/packet/support.py | 53 +++-- .../tests/reviews/packet/test_repository.py | 32 ++- backend/tests/reviews/packet/test_storage.py | 186 ++++++++++++++++-- docs/architecture_data_model.md | 4 +- docs/spec_review_lifecycle.md | 6 +- 10 files changed, 292 insertions(+), 40 deletions(-) diff --git a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md index 0a052915a..1b123f72f 100644 --- a/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md +++ b/.commitrail/initiatives/WS-REV-001/WS-REV-001-03B.md @@ -177,8 +177,9 @@ Temp-table names must not influence constraints. No privileged bypass fixture. `ReviewPacketRepository.store(lease_id, membership)` locks the exact project-qualified REV lease, then its owning queue, before selecting/writing that lease's packet (the existing lease-then-queue order). New insertion requires -active/leased/exact-pointer facts under those locks; replay remains possible -after closure. It +active/leased/exact-pointer facts and an unexpired lease against PostgreSQL +`clock_timestamp()` after those locks; the INSERT guard independently enforces +that deadline. Exact replay remains possible after expiry or closure. It allocates a UUIDv7 only for a new packet, inserts header and all guide rows, and flushes without committing. Deferred checks stay within the caller transaction. An identical retry returns the stored identity; any membership difference raises @@ -188,7 +189,7 @@ project or contributor lock and cannot grant authority. `read(project_id, lease_id)` returns only normalized stored metadata as detached strict facts, ordered by source item order; qualify ownership before lookup. -`ReviewPacketStored` contains `packet_id`, `review_lease_id`, +`ReviewPacketStored` contains `packet_manifest_id` (the existing AUTH name), `review_lease_id`, `review_queue_entry_id`, `packet_manifest_generation`, `packet_manifest_digest`, `created_at`, and the canonical ART `membership` value. A missing or foreign scope returns no packet. Authorization is a future caller's @@ -247,10 +248,17 @@ These tests bind the storage boundary; execution results and review freshness be - `test_packet_matches_canonical_owners_and_digest`: every header/nested owner value and Python/SQL digest parity, including changed ordered membership. - `test_packet_rejects_coherent_owner_substitutions`: independently valid foreign - project, task, Submission/version, run/result, activated setup/generation, ZIP. + project, task, Submission/version, run/result, activated setup/generation, ZIP; + final altered-membership digest, named rejection and result-owner removal probe. - `test_packet_rejects_null_source_fields`: each nonnullable source independently. - `test_packet_creation_time_is_database_owned`: supplied NULL/past/future times. - `test_packet_requires_active_exact_lease`: terminal and successor lease insertions. +- `test_packet_rejects_expired_but_active_lease`: repository and SQL deadline + rejection from a transaction opened before expiry, with a deadline-removal probe. +- `test_exact_packet_replay_survives_expiry_then_closure`: retained replay after + actual deadline passage and after persisted expiry closure. +- `test_child_rejects_uncommitted_parent_that_rolls_back`: two simultaneous + sessions, invisible-parent FK rejection, parent rollback and no retained child. - `test_packet_rejects_coherent_sibling_under_another_lease`: a complete valid same-project sibling packet cannot use another lease; removing only the creation tuple comparison makes the rejection assertion fail. @@ -282,3 +290,25 @@ Mutation probes target the relevant predicate after valid fixture setup: digest, lease status/pointer, committed-upload receipt, exact set, source identity and immutability. Each must reach the negative assertion and fail there when its guard is removed; harness mismatch/setup failure is not discriminating proof. + +### Review corrections within this boundary + +Creation rejects an expired-but-still-active lease even if expiry reconciliation +has not closed it. The repository uses PostgreSQL time after locking and preserves +identical stored replay before checking the deadline; the INSERT trigger checks +the deadline independently. A real elapsed lease, not a forged timestamp or +disabled lease guard, proves both rejections and retained replay. Removing only +the deadline predicate must make the SQL rejection assertion fail. + +Direct-SQL substitution fixtures hash the final altered request, ZIP and guide +member fields. Each negative identifies the intended creation, ownership, FK or +membership failure. A valid foreign result identity with a correct substituted +digest is rejected by its owner equality; removing only that equality must make +the assertion fail while all other constraints remain enforced. + +An independent-session child INSERT races an uncommitted packet parent. The +parent is visible to its writer and invisible to the child session, so PostgreSQL +rejects the child at the named parent FK while the parent's transaction is still +open. Rolling that parent back leaves neither row. This concurrency proof +exercises actual parent visibility; the separate missing-parent validator test +covers its explicit defensive branch. diff --git a/backend/alembic/versions/0012_review_packet.py b/backend/alembic/versions/0012_review_packet.py index 4402af095..90a84afe0 100644 --- a/backend/alembic/versions/0012_review_packet.py +++ b/backend/alembic/versions/0012_review_packet.py @@ -83,6 +83,7 @@ def upgrade() -> None: SELECT * INTO queue_row FROM public.review_queue_entries WHERE id=lease_row.review_queue_entry_id AND project_id=NEW.project_id FOR UPDATE; IF NOT FOUND OR lease_row.status <> 'active' OR queue_row.queue_state <> 'leased' + OR lease_row.expires_at <= pg_catalog.clock_timestamp() OR queue_row.active_lease_id IS DISTINCT FROM lease_row.id OR (NEW.review_queue_entry_id,NEW.task_id,NEW.submission_id,NEW.submission_version, NEW.checker_run_id,NEW.packet_manifest_generation) IS DISTINCT FROM diff --git a/backend/app/modules/reviews/packet/repository.py b/backend/app/modules/reviews/packet/repository.py index ab6ea71b9..51e72897f 100644 --- a/backend/app/modules/reviews/packet/repository.py +++ b/backend/app/modules/reviews/packet/repository.py @@ -2,7 +2,7 @@ from uuid import UUID -from sqlalchemy import select +from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from app.core.hashing import canonical_json_hash @@ -52,9 +52,11 @@ async def store(self, lease_id: UUID, membership: ReviewPacketMembership) -> Rev if existing.membership != membership: raise ReviewPacketConflict() return existing + database_now = await self._session.scalar(select(func.clock_timestamp())) if ( queue is None or lease.status != "active" + or lease.expires_at <= database_now or queue.queue_state != "leased" or queue.active_lease_id != lease.id ): @@ -89,7 +91,7 @@ async def store(self, lease_id: UUID, membership: ReviewPacketMembership) -> Rev ) await self._session.flush() return ReviewPacketStored( - packet_id=packet.id, + packet_manifest_id=packet.id, review_lease_id=lease_id, review_queue_entry_id=queue.id, packet_manifest_generation=packet.packet_manifest_generation, @@ -120,7 +122,7 @@ async def read(self, project_id: UUID, lease_id: UUID) -> ReviewPacketStored | N ) ).all() return ReviewPacketStored( - packet_id=packet.id, + packet_manifest_id=packet.id, review_lease_id=packet.review_lease_id, review_queue_entry_id=packet.review_queue_entry_id, packet_manifest_generation=packet.packet_manifest_generation, diff --git a/backend/app/modules/reviews/packet/schemas.py b/backend/app/modules/reviews/packet/schemas.py index f2b7e3d09..6f4100d75 100644 --- a/backend/app/modules/reviews/packet/schemas.py +++ b/backend/app/modules/reviews/packet/schemas.py @@ -19,7 +19,7 @@ class ReviewPacketStored(BaseModel): model_config = ConfigDict(extra="forbid", frozen=True, strict=True) - packet_id: UUID + packet_manifest_id: UUID review_lease_id: UUID review_queue_entry_id: UUID packet_manifest_generation: StrictInt = Field(ge=1) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 77abf2256..f6c918452 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -23,7 +23,7 @@ DDL_LOCK_DIRECTORY = Path("/tmp") # Match the PostgreSQL 16 engine used by Backend CI. Catalog identity rendering # differs across major versions; regenerate only after comparing actual objects. -EXPECTED_PUBLIC_SCHEMA_SHA256 = "7b79a9331aad5d7fc19fe85fa8c86f8effea39ccdb7c1ed860c1bbdf60098868" +EXPECTED_PUBLIC_SCHEMA_SHA256 = "018585294efdfdaeac5c6849ddb604d5a51970bdc6686a289b5ab34f9202062b" PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", "alembic_version", diff --git a/backend/tests/reviews/packet/support.py b/backend/tests/reviews/packet/support.py index 6fe358681..2cf5a3dc4 100644 --- a/backend/tests/reviews/packet/support.py +++ b/backend/tests/reviews/packet/support.py @@ -1,10 +1,10 @@ """Packets built from real activated guides, admitted ZIPs and executed checker runs.""" from contextlib import asynccontextmanager -from datetime import UTC, datetime, timedelta +from datetime import timedelta from uuid import UUID -from sqlalchemy import select, text +from sqlalchemy import func, select, text from app.core.identifiers import new_record_id from app.modules.artifacts.api.review_packet import ( @@ -32,13 +32,15 @@ @asynccontextmanager -async def packet_source(tmp_path, database_url, **source_options): +async def packet_source( + tmp_path, database_url, *, lease_duration=timedelta(days=1), **source_options +): async with completed_source(tmp_path, database_url, **source_options) as h: - await prepare_packet(h) + await prepare_packet(h, lease_duration=lease_duration) yield h -async def prepare_packet(h): +async def prepare_packet(h, *, lease_duration=timedelta(days=1)): """Attach canonical packet membership and a committed lease to a completed source.""" async with h.factory() as session: submission = await session.get(Submission, str(h.request.submission_id)) @@ -113,6 +115,7 @@ async def prepare_packet(h): ) await session.commit() actor_id = await _human_actor(session, label="packet-reviewer") + database_now = await session.scalar(select(func.clock_timestamp())) lease = await repository.add_lease( ReviewLeaseInput( id=new_record_id(), @@ -124,7 +127,7 @@ async def prepare_packet(h): reviewer_id=actor_id, reviewer_contribution_policy_version_id=h.source["contribution_policy_version_id"], attempt_generation=1, - expires_at=datetime.now(UTC) + timedelta(days=1), + expires_at=database_now + lease_duration, ) ) queue.queue_state = "leased" @@ -142,7 +145,6 @@ async def raw_insert(session, h, *, header=None, members=None): review_lease_id=h.lease_id, review_queue_entry_id=h.queue_id, packet_manifest_generation=1, - packet_manifest_digest=canonical_json_hash(h.membership.model_dump(mode="json")), **h.membership.request.model_dump(), submission_binding_id=h.membership.submission.binding_id, submission_logical_role=h.membership.submission.logical_role, @@ -151,14 +153,21 @@ async def raw_insert(session, h, *, header=None, members=None): import json actual_members = h.membership.guide_documents if members is None else members - body = h.membership.model_dump(mode="json") + values.update(header or {}) + body = { + "request": {key: values[key] for key in ReviewPacketMembershipRequest.model_fields}, + "submission": { + "binding_id": values["submission_binding_id"], + "logical_role": values["submission_logical_role"], + "media_type": values["submission_media_type"], + }, + } body["guide_documents"] = [ m.model_dump(mode="json") if hasattr(m, "model_dump") else m for m in actual_members ] - values["packet_manifest_digest"] = canonical_json_hash( - json.loads(json.dumps(body, default=str)) + values["packet_manifest_digest"] = (header or {}).get( + "packet_manifest_digest", canonical_json_hash(json.loads(json.dumps(body, default=str))) ) - values.update(header or {}) await session.execute( text( "INSERT INTO public.review_packet_manifests (" @@ -185,3 +194,25 @@ async def raw_insert(session, h, *, header=None, members=None): item, ) return values["id"] + + +async def wait_for_lease_expiry(session, lease_id): + """Wait for the actual database deadline without rewriting immutable lease facts.""" + await session.execute( + text( + "SELECT pg_sleep(GREATEST(0, EXTRACT(EPOCH FROM " + "(expires_at-clock_timestamp())))::double precision + 0.01) " + "FROM public.review_leases WHERE id=:id" + ), + {"id": lease_id}, + ) + assert ( + await session.scalar( + text( + "SELECT status='active' AND expires_at<=clock_timestamp() " + "FROM public.review_leases WHERE id=:id" + ), + {"id": lease_id}, + ) + is True + ) diff --git a/backend/tests/reviews/packet/test_repository.py b/backend/tests/reviews/packet/test_repository.py index eda3c63c1..6f6949f2e 100644 --- a/backend/tests/reviews/packet/test_repository.py +++ b/backend/tests/reviews/packet/test_repository.py @@ -18,7 +18,7 @@ async def test_packet_matches_canonical_owners_and_digest(tmp_path, clean_postgr stored = await repository.store(h.lease_id, h.membership) await session.commit() assert set(stored.model_dump()) == { - "packet_id", + "packet_manifest_id", "review_lease_id", "review_queue_entry_id", "packet_manifest_generation", @@ -264,3 +264,33 @@ async def test_packet_retains_superseded_guide_lineage(tmp_path, clean_postgres_ ) assert await ReviewPacketRepository(session).store(h.lease_id, h.membership) == original await session.commit() + + +@pytest.mark.asyncio +async def test_exact_packet_replay_survives_expiry_then_closure(tmp_path, clean_postgres_database): + from datetime import timedelta + from tests.reviews.packet.support import wait_for_lease_expiry + + async with packet_source( + tmp_path, clean_postgres_database, lease_duration=timedelta(seconds=5) + ) as h: + async with h.factory() as session: + stored = await ReviewPacketRepository(session).store(h.lease_id, h.membership) + await session.commit() + await wait_for_lease_expiry(session, h.lease_id) + assert await ReviewPacketRepository(session).store(h.lease_id, h.membership) == stored + await session.execute( + text( + "UPDATE public.review_leases SET status='expired',close_reason='lease_expired',closed_at=clock_timestamp() WHERE id=:id" + ), + {"id": h.lease_id}, + ) + await session.execute( + text( + "UPDATE public.review_queue_entries SET queue_state='pending',active_lease_id=NULL WHERE id=:id" + ), + {"id": h.queue_id}, + ) + await session.commit() + assert await ReviewPacketRepository(session).store(h.lease_id, h.membership) == stored + await session.commit() diff --git a/backend/tests/reviews/packet/test_storage.py b/backend/tests/reviews/packet/test_storage.py index 4e98595d8..bc6f77a13 100644 --- a/backend/tests/reviews/packet/test_storage.py +++ b/backend/tests/reviews/packet/test_storage.py @@ -236,9 +236,13 @@ async def test_packet_shadow_tables_cannot_change_custody(tmp_path, clean_postgr async def test_packet_rejects_numeric_version_and_generation(tmp_path, clean_postgres_database): async with packet_source(tmp_path, clean_postgres_database) as h: await valid_control(h) - for field in ("submission_version", "packet_manifest_generation", "setup_generation"): + for field, expected_error in ( + ("submission_version", "review packet requires exact active lease"), + ("packet_manifest_generation", "review packet requires exact active lease"), + ("setup_generation", "fk_review_packet_setup"), + ): async with h.factory() as session: - with pytest.raises(DBAPIError): + with pytest.raises(DBAPIError, match=expected_error): await raw_insert(session, h, header={field: 999}) await session.commit() await session.rollback() @@ -256,38 +260,61 @@ async def test_packet_rejects_coherent_owner_substitutions(tmp_path, clean_postg await valid_control(h) await valid_control(foreign) foreign_header = foreign.membership.request.model_dump() - for field in ( - "project_id", - "task_id", - "submission_id", - "checker_run_id", - "result_id", - "guide_id", - "source_snapshot_id", - "project_setup_run_id", + for field, expected_error in ( + ("project_id", "review packet lease unavailable"), + ("task_id", "review packet requires exact active lease"), + ("submission_id", "review packet requires exact active lease"), + ("checker_run_id", "review packet requires exact active lease"), + ("result_id", "review packet canonical header mismatch"), + ("guide_id", "fk_review_packet_snapshot"), + ("source_snapshot_id", "fk_review_packet_snapshot"), + ("project_setup_run_id", "fk_review_packet_setup"), ): assert getattr(h.membership.request, field) != foreign_header[field] async with h.factory() as session: - with pytest.raises(DBAPIError): + with pytest.raises(DBAPIError, match=expected_error): await raw_insert(session, h, header={field: foreign_header[field]}) await session.commit() await session.rollback() - for header, members in ( - ({"submission_binding_id": foreign.membership.submission.binding_id}, None), - ({}, foreign.membership.guide_documents), + for header, members, expected_error in ( + ( + {"submission_binding_id": foreign.membership.submission.binding_id}, + None, + "canonical header mismatch", + ), + ({}, foreign.membership.guide_documents, "canonical guide membership"), ( {}, ( *h.membership.guide_documents, foreign.membership.guide_documents[0].model_copy(update={"item_order": 10}), ), + "canonical guide membership", ), ): async with h.factory() as session: - with pytest.raises(DBAPIError): + with pytest.raises(DBAPIError, match=expected_error): await raw_insert(session, h, header=header, members=members) await session.commit() await session.rollback() + # A correct substituted digest must not mask a missing result-owner equality. + async with h.factory() as session: + definition = await session.scalar( + text( + "SELECT pg_get_functiondef('public.validate_review_packet(uuid)'::regprocedure)" + ) + ) + predicate = "AND r.result_id=packet.result_id" + assert definition.count(predicate) == 1 + await session.execute(text(definition.replace(predicate, ""))) + with pytest.raises(pytest.fail.Exception, match="DID NOT RAISE"): + await assert_packet_rejected( + session, + h, + header={"result_id": foreign.membership.request.result_id}, + message="review packet canonical header mismatch", + ) + await session.rollback() # Stored foreign rows, not nonexistent UUIDs, exercise concealed reads. from app.modules.reviews.packet.repository import ReviewPacketRepository @@ -625,3 +652,130 @@ async def substitute(): await session.scalar(text("SELECT count(*) FROM public.review_packet_manifests")) == 0 ) + + +@pytest.mark.asyncio +async def test_packet_rejects_expired_but_active_lease(tmp_path, clean_postgres_database): + from datetime import timedelta + from app.modules.artifacts.api.review_packet import ReviewPacketMembershipUnavailable + from app.modules.reviews.packet.repository import ReviewPacketRepository + from tests.reviews.packet.support import wait_for_lease_expiry + + async with packet_source( + tmp_path, clean_postgres_database, lease_duration=timedelta(seconds=5) + ) as h: + await valid_control(h) + async with h.factory() as session: + # Begin before expiry: transaction_timestamp() would incorrectly remain valid. + assert ( + await session.scalar( + text( + "SELECT transaction_timestamp()