From 8158af4a978ec774f503aa6972fcec0d346ed653 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sat, 1 Aug 2026 14:58:40 +0100 Subject: [PATCH 1/7] docs(xint): split immutable policy lineage from activation --- ...H-001-12D2-guide-bound-policy-mutations.md | 15 +- ...-03P-review-revision-policy-persistence.md | 9 +- .../ACTION_CUSTODY.md | 8 +- .../CHUNK_MAP.md | 9 +- .../WS-XINT-003-rev-auth-end-to-end/PLAN.md | 8 +- .../WS-XINT-003-rev-auth-end-to-end/STATUS.md | 3 +- ...-XINT-003-02-policy-mutation-activation.md | 97 ++++++++++- ...WS-XINT-003-02A-policy-identity-lineage.md | 155 ++++++++++++++++++ ...XINT-003-02B-policy-mutation-activation.md | 37 +++++ ...S-XINT-003-02A-preimplementation-review.md | 30 ++++ 10 files changed, 341 insertions(+), 30 deletions(-) create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-preimplementation-review.md diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md index 260919731..dcf32bbf3 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md @@ -15,15 +15,18 @@ Add separately authorized routes for review and revision policy records after This contract is reconciled with REV-03P by WS-XINT-003-01. REV owns the immutable/versioned policy semantics; AUTH owns the mutation authorization, -PREP consumption, and decision evidence. Chunk WS-XINT-003-02 implements the -single path below; neither parent contract may build an alternate writer. +PREP consumption, and decision evidence. WS-XINT-003-02A first installs exact +immutable policy identity and downstream lineage without activation; 02B then +implements the single writer path below. Neither parent contract may build an +alternate writer. ## One writer path - Surviving API: separate `PUT /projects/{project_id}/review-policy` and - `PUT /projects/{project_id}/revision-policy` routes in - `backend/app/modules/projects/router.py`, each declaring its exact primary - ActionId. + `PUT /projects/{project_id}/revision-policy` routes in the dedicated + `backend/app/modules/projects/policy_mutation_router.py`, registered once by + `backend/app/api/router.py`, each declaring its exact primary ActionId. This + follows the project-create and guide-mutation router boundary on current main. - Surviving service: new `ProjectPolicyMutationService` methods `replace_review_policy()` and `replace_revision_policy()`. - Surviving repository: new append-only @@ -31,6 +34,8 @@ single path below; neither parent contract may build an alternate writer. `ProjectRepository.add_revision_policy_version()` methods over the existing `ReviewPolicy` and `RevisionPolicy` tables/models, upgraded as necessary for immutable version provenance. +- A separate `PolicyMutationReplayRepository` may own only the idempotency + ledger. It must not read or write either policy table. - Retired callable mutators: `ProjectRepository.upsert_review_policy()`, `ProjectRepository.upsert_revision_policy()`, `ProjectService._review_policy_model()`, and diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md index 3e3b361b2..bd34d6dcc 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md @@ -2,7 +2,7 @@ ## Status -Reconciled planning input to `WS-XINT-003-02`. It is not an independent +Reconciled planning input to `WS-XINT-003-02A` and `02B`. It is not an independent implementation path. ## Goal @@ -12,7 +12,8 @@ later routing, lease, decision, and human revision behavior. REV owns field semantics, version identity, draft/active immutability, and the facts later lifecycle code consumes. AUTH-12D2 owns authority, PREP, evidence, -and the only mutation surface. The shared implementation is XINT-003-02. +and the only mutation surface. XINT-003-02A owns immutable identity/lineage; +02B owns the shared mutation activation. ## Canonical persistence path @@ -41,7 +42,7 @@ L1: policy immutability, duration/limit semantics, and later decision authority. The exact current-main project models, migration, policy schemas, canonical writer service/repository, AUTH PREP integration, focused tests, and initiative -evidence must be fixed in the refreshed XINT-003-02 contract. +evidence must be fixed in the refreshed XINT-003-02A/02B contracts. ## Not allowed @@ -78,4 +79,4 @@ reuse/dedup, docs, test-delta, and CI integrity. ## Stop Do not implement this parent contract independently. Refresh and implement only -WS-XINT-003-02 after an explicit user request. +WS-XINT-003-02A/02B after an explicit user request. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md index db4603577..67f8496e8 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md @@ -6,8 +6,8 @@ This table is the planning source of truth for the v0.1 review and human-revisio | ActionId | PermissionId | Principal and scope | Resource family | Surface owner | State | Activation wave | |---|---|---|---|---|---|---| -| `project.review_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + ReviewPolicy version | project/REV semantics; AUTH mutation | registered planned | `WS-XINT-003-02` | -| `project.revision_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + RevisionPolicy version | project/REV semantics; AUTH mutation | registered planned | `WS-XINT-003-02` | +| `project.review_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + ReviewPolicy version | project/REV semantics; AUTH mutation | registered planned | `WS-XINT-003-02B` after 02A lineage | +| `project.revision_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + RevisionPolicy version | project/REV semantics; AUTH mutation | registered planned | `WS-XINT-003-02B` after 02A lineage | | `review.queue.read` | `review.queue.read` | reviewer grant; exact project; self-review denied | concealed current-work view | REV | registered planned | `WS-XINT-003-03A` | | `review.claim` | `review.claim` | reviewer grant; exact project; self-review denied | queue entry + global reviewer lease state | REV | registered planned | `WS-XINT-003-03A` | | `review.release` | `review.release` | owning reviewer and active lease | ReviewLease | REV | registered planned | `WS-XINT-003-03A` | @@ -64,8 +64,8 @@ These actions are not XINT-003 custody. Generic artifact download, adjudication, | ActionId | Exact prerequisite behavior/manifest | |---|---| -| `project.review_policy.update` | existing policy records plus refreshed REV-03P/AUTH-12D2 contract; implemented only by `WS-XINT-003-02` | -| `project.revision_policy.update` | existing policy records plus refreshed REV-03P/AUTH-12D2 contract; implemented only by `WS-XINT-003-02` | +| `project.review_policy.update` | 02A immutable identity/lineage plus refreshed REV-03P/AUTH-12D2; activated only by `WS-XINT-003-02B` | +| `project.revision_policy.update` | 02A immutable identity/lineage plus refreshed REV-03P/AUTH-12D2; activated only by `WS-XINT-003-02B` | | `review.queue.read` | merged hidden REV-05 concealed current-work view | | `review.claim` | merged hidden REV-05 queue admission and REV-06 atomic lease behavior | | `review.release` | merged hidden REV-06 lease release behavior | diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/CHUNK_MAP.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/CHUNK_MAP.md index ada054799..febf2ce75 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/CHUNK_MAP.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/CHUNK_MAP.md @@ -3,8 +3,9 @@ | Chunk | Purpose | Risk | Dependency | |---|---|---|---| | `WS-XINT-003-01` | Reconcile policy ownership, complete REV catalogue, permissions, surfaces, resource families, and fixed-service matrix while actions stay planned. | L1 | approved plan | -| `WS-XINT-003-02` | Join REV policy semantics/persistence to AUTH-12D2 prepared policy mutation cutover and remove duplicate writer paths. | L1 | 01 plus refreshed REV-03P/AUTH-12D2 | -| `WS-XINT-003-03A` | Activate concealed reviewer current-work plus claim/release/preference with exact project grant, self-review denial, global lease limit, and atomic lease/packet-manifest freeze. | L1 | 02 plus hidden REV queue/lease behavior | +| `WS-XINT-003-02A` | Cut policy persistence and Task/Submission/Checker locks from guide-version aliases to immutable policy-version identity; activate nothing. | L1 | 01 plus refreshed REV-03P/AUTH-12D2 | +| `WS-XINT-003-02B` | Activate the sole review/revision policy mutation service through AUTH PREP after immutable lineage exists. | L1 | merged 02A | +| `WS-XINT-003-03A` | Activate concealed reviewer current-work plus claim/release/preference with exact project grant, self-review denial, global lease limit, and atomic lease/packet-manifest freeze. | L1 | 02B plus hidden REV queue/lease behavior | | `WS-XINT-003-03B` | Activate preference and lease expiry fixed services only. | L1 | 03A plus hidden timer behavior | | `WS-XINT-003-04` | Activate human `review.context.read` and reviewer finding evidence while consuming XINT-002-07A's ART-only packet/materialization/binding capability. | L1 | 03B plus hidden REV packet/evidence manifests and XINT-002-07A | | `WS-XINT-003-05` | Activate only bounded `review.chain.read`, consuming the active REV context and XINT-002 packet/materialization boundary. | L1 | 04 plus merged XINT-002-07A | @@ -13,9 +14,9 @@ | `WS-XINT-003-08R` | Register four missing privileged recovery/lifecycle ActionIds as planned with complete catalogue/migration parity; activate nothing. | L1 | 07 plus exact hidden-feature registration manifests | | `WS-XINT-003-08A` | Activate Project Manager and Operator queue/revision recovery commands with exact scope and reasons. | L1 | 08R plus hidden REV recovery behavior | | `WS-XINT-003-08B` | Activate both identities for the single `review.reconcile.run` ActionId together, plus artifact-reference, projection, and lifecycle-control surfaces. | L1 | 08A plus hidden REV jobs/projection/control | -| `WS-XINT-003-09` | Prove end-to-end least privilege, revocation, replay, concurrency, atomicity, artifact isolation, and coherent route release. | L1 | 02-08B | +| `WS-XINT-003-09` | Prove end-to-end least privilege, revocation, replay, concurrency, atomicity, artifact isolation, and coherent route release. | L1 | 02A-08B | -Chunks 02 through 09 are planning skeletons, not implementation-ready contracts, +Chunks 02A through 09 are planning skeletons, not implementation-ready contracts, until refreshed on current main with exact allowed files and commands. Each row maps to one PR unless its current-main contract is split into smaller children before implementation. A split cannot add a new permission, action, diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/PLAN.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/PLAN.md index 2afadd811..249464cc5 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/PLAN.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/PLAN.md @@ -7,8 +7,10 @@ activation waves: 1. Reconcile policy ownership, the complete catalogue, permissions, principal classes, fixed-service matrix, surface manifests, and planned availability. -2. Cut over review/revision policy configuration through one persistence path: - REV owns semantics; AUTH-12D2 owns authorization and PREP consumption. +2. First cut policy persistence and every downstream lock to immutable policy + identity in 02A without runtime activation. Then cut over review/revision + policy configuration through one persistence path in 02B: REV owns + semantics; AUTH-12D2 owns authorization and PREP consumption. 3. Activate concealed reviewer current-work, claim/release/preference, and timer services only after REV queue/lease behavior exists. 4. Amend XINT-002-07 into two ART-only owner waves: 07A is the only ActionId @@ -106,7 +108,7 @@ hidden behavior are rejected. ## Stop boundary This planning amendment creates no runtime code and activates no action. Chunks -02 through 09 are non-implementable planning skeletons until a current-main +02A through 09 are non-implementable planning skeletons until a current-main refresh replaces every file/command placeholder with exact boundaries and the user explicitly requests that chunk. Planning complete does not start runtime work. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md index cf21b03c8..83bb7b2b8 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md @@ -31,4 +31,5 @@ REV-owned semantics with AUTH-owned mutation authorization. ## Next step Keep hosted exact-head gates green, resolve all external review, and obtain -human merge. Then stop before runtime policy work in WS-XINT-003-02. +human merge. Parent 02 plan review then required 02A immutable identity/lineage +before 02B runtime policy mutation activation. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02-policy-mutation-activation.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02-policy-mutation-activation.md index 4eef0b96b..c3c5142ad 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02-policy-mutation-activation.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02-policy-mutation-activation.md @@ -2,9 +2,11 @@ ## Status -Non-implementable planning skeleton after 01. Before implementation, refresh on -current main with exact allowed files and commands, then require an explicit -user request for this chunk. +Superseded before implementation by `WS-XINT-003-02A` and +`WS-XINT-003-02B`. Current-main plan review proved that immutable policy +versions cannot be introduced safely while Task, Submission, and CheckerRun +still use guide version as policy identity. This record is retained only as the +rejected combined design input and is not an implementation path. ## Goal @@ -17,9 +19,51 @@ L1 policy and authorization mutation. ## Allowed files -Must be enumerated exactly at current-main start. Only policy-owned project/REV -models, repository/service/routes, AUTH typed contexts/catalogue parity, -migration, focused tests, docs, and this initiative's evidence may be included. +```text +backend/app/api/router.py +backend/app/modules/projects/models.py +backend/app/modules/projects/repository.py +backend/app/modules/projects/schemas.py +backend/app/modules/projects/service.py +backend/app/modules/projects/policy_mutation_replay_repository.py +backend/app/modules/projects/policy_mutation_service.py +backend/app/modules/projects/policy_mutation_router.py +backend/app/modules/authorization/catalogue.py +backend/app/modules/authorization/kernel.py +backend/app/modules/authorization/prepared.py +backend/app/modules/authorization/runtime.py +backend/alembic/versions/0046_review_revision_policy_authority.py +backend/tests/test_authorization.py +backend/tests/test_project_policy_mutations.py +backend/tests/test_projects.py +backend/tests/test_alembic.py +backend/scripts/api_contract_e2e.py +docs/spec_authorization_service.md +docs/spec_review_lifecycle.md +.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md +.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02-policy-mutation-activation.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02-preimplementation-review.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02-internal-review.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02-pr-trust-bundle.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02-external-review-response.md +``` + +The dedicated policy mutation router/service and replay-only repository follow +the existing project-create and guide-mutation boundaries. The replay +repository may touch only the policy-mutation idempotency ledger and must not +read, insert, update, or delete `ReviewPolicy` or `RevisionPolicy` rows. +`ProjectRepository.add_review_policy_version()` and +`add_revision_policy_version()` are the only policy-table write primitives; +its policy read/lock/append methods remain internal and are not an independently +callable authorization path. + +AUTH edits are limited to activating these two existing ActionIds and enforcing +their already-typed resource contexts through the existing PREP protocol. This +chunk may not reshape the general kernel, prepared-capability protocol, or any +unrelated action evaluator. ## Not allowed @@ -43,9 +87,44 @@ reputation, frontend, duplicate policy tables, or legacy writer compatibility. ## Verification -Focused PostgreSQL policy/authorization/migration/concurrency tests, Ruff, -90-percent changed-subsystem coverage, hosted full coverage, API contract proof, -and all required L1 reviewers. +```bash +(cd backend && .venv/bin/python -m ruff check app tests scripts) +(cd backend && WORKSTREAM_TEST_ADMIN_DATABASE_URL="$WORKSTREAM_TEST_ADMIN_DATABASE_URL" \ + .venv/bin/python scripts/run_isolated_tests.py \ + --metadata-json .ci/xint-003-02-focused.json --lane xint_003_02 \ + -- .venv/bin/pytest -q tests/test_authorization.py \ + tests/test_project_policy_mutations.py tests/test_projects.py tests/test_alembic.py) +(cd backend && WORKSTREAM_TEST_DATABASE_URL="$WORKSTREAM_TEST_DATABASE_URL" \ + .venv/bin/pytest -q tests/test_project_policy_mutations.py \ + --cov=app.modules.projects.policy_mutation_replay_repository \ + --cov=app.modules.projects.policy_mutation_service \ + --cov=app.modules.projects.policy_mutation_router \ + --cov-report=term-missing --cov-fail-under=90) +(cd backend && .venv/bin/python scripts/api_contract_e2e.py) +python3 scripts/check_stale_authorization_docs.py +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_markdown_links.py +git diff --check +``` + +PostgreSQL tests must cover migration upgrade, downgrade/re-upgrade, direct SQL +update/delete refusal, crossed replacements, rollback, stale authority and +policy lineage, and exactly-once replay. GitHub `Backend / test` supplies the +repository-wide 78-percent full-suite gate; materially changed policy-mutation +files must remain at or above 90 percent. `Agent Gates / agent-gates` and +CodeRabbit must pass on the exact final PR head. + +## Required reviewers + +Architecture, security/auth, product/operations, QA/test, senior engineering, +reuse/dedup, docs, test-delta, and CI integrity. + +## Human review focus + +Confirm the sole writer path, immutable-version semantics, exact Project +Manager/project/guide binding, active-guide freeze, no compatibility path, and +absence of review-lifecycle activation. ## Stop condition diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md new file mode 100644 index 000000000..b3d230818 --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md @@ -0,0 +1,155 @@ +# Chunk Contract: WS-XINT-003-02A — Immutable Policy Identity And Lineage + +## Status + +Implementation-ready candidate refreshed from `main` at `ad8da7e5`. Runtime +edits require L1 plan-review PASS. The user's start of parent 02 authorizes this +first child only; 02B does not begin automatically. + +## Goal + +Make the existing ReviewPolicy and RevisionPolicy tables express immutable, +append-only versions and make ProjectGuide, Task, Submission, and CheckerRun +lock exact policy identity instead of treating guide version as policy version. +No public mutation route or authorization action becomes available. + +## Risk class + +L1 policy identity and cross-subsystem data lineage. + +## Allowed files + +```text +backend/app/modules/projects/models.py +backend/app/modules/projects/repository.py +backend/app/modules/projects/schemas.py +backend/app/modules/projects/service.py +backend/app/modules/projects/authorization_reads.py +backend/app/modules/projects/policy_lineage.py +backend/app/modules/tasks/models.py +backend/app/modules/tasks/repository.py +backend/app/modules/tasks/schemas.py +backend/app/modules/tasks/service.py +backend/app/modules/checkers/models.py +backend/app/modules/checkers/schemas.py +backend/app/modules/checkers/service.py +backend/app/modules/checkers/runner.py +backend/alembic/versions/0046_immutable_review_revision_policy_lineage.py +backend/tests/test_projects.py +backend/tests/test_tasks.py +backend/tests/test_checkers.py +backend/tests/test_alembic.py +backend/tests/test_artifact_admission.py +backend/tests/test_policy_identity_lineage.py +docs/spec_review_lifecycle.md +.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md +.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/CHUNK_MAP.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/PLAN.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02-policy-mutation-activation.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-preimplementation-review.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md +``` + +## Not allowed + +Public policy mutation routes, ActionId activation, PREP consumption, grants, +queue/lease/Review/finding/revision execution, ART behavior, CON, payment, +reputation, frontend, duplicate policy tables, inferred lease/preference values, +or compatibility aliases for guide-version-as-policy-version fields. + +## Acceptance criteria + +- Existing `ReviewPolicy` and `RevisionPolicy` tables become immutable + multi-version records for one exact ProjectGuide; no duplicate policy table + or mutable current-row path exists. +- ProjectGuide selects the exact current review-policy and revision-policy row. + Draft selection may advance only through the later 02B writer; activation + freezes the selected identities. Existing guides with unambiguous policy rows + are backfilled to those exact IDs. A new draft created between 02A and 02B has + nullable selections, and policy readiness/activation fails closed until 02B + installs complete selected versions; 02A does not modify the guide writer. +- Review policy types explicitly represent positive preference-window and + lease-duration values, capacity fixed to one in v0.1, + `self_review_allowed = false`, + close-task rejection, finding-evidence requirement, and allowed decisions. + None is inferred from legacy `sla_hours`. +- Revision policy explicitly represents positive revision limit and deadline + semantics and the permitted resubmission/reassignment rules. Reaching a limit + or deadline blocks preparation; it never auto-rejects or auto-closes a Task. + The legacy `auto_reject_after_limit` field is removed from model, schema, and + storage with no compatibility alias, and its historical value is not treated + as lifecycle authority. +- Task locks exact review/revision policy IDs plus immutable generation/digest; + Submission and CheckerRun copy and FK-chain those exact facts. Guide version + remains guide lineage only and is not a policy identifier. +- Historical rows migrate deterministically without inventing lease/preference + meaning. Storage distinguishes `complete` policy semantics from typed + `legacy_incomplete` rows: complete rows require positive preference/lease and + revision values, while migrated missing values remain nullable only on + `legacy_incomplete` rows. Incomplete rows remain readable but the canonical + lineage/readiness predicate must reject them for future review activation. +- PostgreSQL rejects policy update/delete/truncate and rejects any mismatch + across project, guide, selected policy, Task, Submission, or CheckerRun. +- All existing task/submission/checker behavior remains otherwise unchanged, + and both policy mutation ActionIds remain planned/unavailable. + +## Verification + +```bash +(cd backend && .venv/bin/python -m ruff check app tests scripts) +(cd backend && install -d -m 700 .ci/xint-003-02a && \ + WORKSTREAM_TEST_ADMIN_DATABASE_URL="$WORKSTREAM_TEST_ADMIN_DATABASE_URL" \ + .venv/bin/python scripts/run_isolated_tests.py \ + --metadata-json .ci/xint-003-02a/focused.json --lane xint_003_02a \ + -- .venv/bin/pytest -q tests/test_policy_identity_lineage.py \ + tests/test_alembic.py -k xint003_02a) +(cd backend && install -d -m 700 .ci/xint-003-02a && \ + WORKSTREAM_TEST_ADMIN_DATABASE_URL="$WORKSTREAM_TEST_ADMIN_DATABASE_URL" \ + .venv/bin/python scripts/run_isolated_tests.py \ + --metadata-json .ci/xint-003-02a/coverage.json --lane xint_003_02a_coverage \ + -- .venv/bin/pytest -q tests/test_policy_identity_lineage.py \ + --cov=app.modules.projects.policy_lineage --cov-branch \ + --cov-report=term-missing --cov-fail-under=90) +python3 scripts/check_stale_authorization_docs.py +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_stale_review_contracts.py +python3 scripts/check_markdown_links.py +git diff --check +``` + +The named tests must cover migration upgrade, downgrade/re-upgrade, historical +backfill, update/delete/truncate refusal, exact ProjectGuide/Task/Submission/ +CheckerRun lineage, rollback, and both actions remaining unavailable. Focused +tests in both named files must use `xint003_02a` in their test node names so the +required keyword selection cannot silently omit new proof. They must include a +migrated incomplete-policy case proving the row remains readable and explicitly +marked `legacy_incomplete` while the canonical readiness predicate denies its +use. Focused +project/task/checker regression selections may be added by exact test node ID as +implementation reveals affected existing cases; they may not expand the +allowed test files. GitHub `Backend / test` supplies the repository-wide +78-percent suite and coverage gate and must demonstrate that materially changed +existing subsystems do not regress. Agent Gates and CodeRabbit must pass on the +exact final head. + +## Required reviewers + +Architecture, security/auth, product/operations, QA/test, senior engineering, +reuse/dedup, docs, test-delta, and CI integrity. + +## Human review focus + +Exact policy identity, safe historical migration, no invented policy semantics, +downstream lock stability, database immutability, and zero runtime activation. + +## Stop condition + +Merge and stop before 02B mutation activation. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md new file mode 100644 index 000000000..6d114da6b --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md @@ -0,0 +1,37 @@ +# Chunk Contract: WS-XINT-003-02B — Policy Mutation Activation + +## Status + +Planning skeleton. Refresh only after 02A merges; do not start automatically. + +## Goal + +Expose the sole review/revision policy writer and activate exactly +`project.review_policy.update` and `project.revision_policy.update` through the +existing opaque, transaction-bound PREP protocol. + +## Required boundary + +- `ProjectPolicyMutationService` is the sole orchestration path. +- `ProjectRepository.add_review_policy_version()` and + `add_revision_policy_version()` are the only policy-table writers. +- A replay-only repository may touch only the idempotency ledger. +- Exact committed replay returns the recorded response without new PREP, + policy write, or allowed evidence, including after later grant/link + revocation. Changed or pending replay conflicts without product state. +- Final PREP consumption follows locks on the exact project, draft guide, + selected current policy, reserved replacement identity, actor/link/grant, + operation, request digest, session, and transaction. +- New versions persist actor, identity link, matched grant, project scope, + ActionId, decision-event reference, predecessor identity/digest, generation, + and canonical policy digest atomically with selection advancement. +- Active/stale guide, stale selected policy, revoked authority, copied/wrong + handle, wrong actor/action/project/guide/policy, replay, and crossed + replacement races fail with no partial policy or audit state. +- No review lifecycle ActionId or behavior is activated. + +## Risk, review, and stop + +L1. Exact files and commands must be refreshed from post-02A main. Require all +L1 reviewers, hosted full coverage, CodeRabbit, and human merge. Merge and stop +before 03A. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-preimplementation-review.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-preimplementation-review.md new file mode 100644 index 000000000..f548d486a --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-preimplementation-review.md @@ -0,0 +1,30 @@ +# Preimplementation Review: WS-XINT-003-02A + +## Result + +Parent `WS-XINT-003-02` failed L1 architecture, security, product/operations, +and QA plan review before runtime edits. It is superseded by 02A and 02B. + +## Blocking findings incorporated + +- Existing unique `(project_id, guide_version)` policy rows cannot express + immutable replacements for one draft guide. +- Task, Submission, and CheckerRun incorrectly use guide version as policy + identity; activation before lineage repair would be unsafe. +- A new generic policy repository risked duplicating the sole writer. 02B may + add only a replay-ledger repository; `ProjectRepository` remains the only + policy-table persistence owner. +- Draft-only final consumption, active-guide freeze, exact provenance, committed + replay recovery, and typed REV policy semantics were not strong enough. +- Focused test, coverage, and API-contract commands were not fully isolated or + complete. + +## Corrective boundary + +02A owns immutable policy identity, explicit typed semantics, guide selection, +and exact Task/Submission/CheckerRun locks while both actions stay unavailable. +02B later owns the sole public writer, PREP, replay custody, provenance, and +activation of exactly two actions. + +The refreshed 02A candidate requires a new focused L1 plan-review pass before +implementation. From 87f334342825b45092564dbf46708cd2e2774703 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sat, 1 Aug 2026 16:57:46 +0100 Subject: [PATCH 2/7] feat(policy): enforce immutable policy identity lineage --- .../REVIEW_LOG.md | 10 + .../WS-XINT-003-rev-auth-end-to-end/STATUS.md | 23 +- ...WS-XINT-003-02A-policy-identity-lineage.md | 9 + .../WS-XINT-003-02A-internal-review.md | 44 ++ .../WS-XINT-003-02A-pr-trust-bundle.md | 67 ++ ...mmutable_review_revision_policy_lineage.py | 597 ++++++++++++++++++ backend/app/modules/checkers/models.py | 62 +- backend/app/modules/checkers/runner.py | 21 +- backend/app/modules/checkers/schemas.py | 8 +- backend/app/modules/checkers/service.py | 111 ++-- backend/app/modules/projects/models.py | 172 ++++- .../app/modules/projects/policy_lineage.py | 70 ++ backend/app/modules/projects/repository.py | 126 ++-- backend/app/modules/projects/schemas.py | 26 +- backend/app/modules/projects/service.py | 298 ++++----- backend/app/modules/tasks/models.py | 136 +++- backend/app/modules/tasks/schemas.py | 32 +- backend/app/modules/tasks/service.py | 127 ++-- backend/scripts/api_contract_e2e.py | 106 +++- backend/scripts/week2_api_e2e.py | 79 ++- backend/tests/conftest.py | 4 +- backend/tests/test_alembic.py | 281 ++++++++- backend/tests/test_artifact_admission.py | 94 ++- backend/tests/test_checkers.py | 68 +- backend/tests/test_policy_identity_lineage.py | 126 ++++ backend/tests/test_projects.py | 305 +++++---- backend/tests/test_tasks.py | 152 ++++- docs/architecture_data_model.md | 72 ++- docs/glossary.md | 3 +- ...ec_chunk_5_submission_packet_foundation.md | 8 +- docs/spec_chunk_6_checker_contract_records.md | 14 +- docs/spec_review_lifecycle.md | 22 +- docs/template_project_guide.md | 17 +- .../terminal_benchmark_api_e2e.py | 24 +- 34 files changed, 2639 insertions(+), 675 deletions(-) create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md create mode 100644 backend/alembic/versions/0046_immutable_review_revision_policy_lineage.py create mode 100644 backend/app/modules/projects/policy_lineage.py create mode 100644 backend/tests/test_policy_identity_lineage.py diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md index 01a847e46..4b649c11b 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md @@ -64,3 +64,13 @@ findings corrected fixed-service identity drift, ART global-matrix wording, runtime owner versus sub-wave ambiguity, missing per-action dependencies, 07B/human activation order, and obsolete signed-start gates. All tracks passed; the final evidence is in `reviews/WS-XINT-003-01-internal-review.md`. + +## WS-XINT-003-02A immutable policy identity and lineage + +Architecture, security/auth, product/operations, QA/test, senior engineering, +reuse/dedup, docs, test-delta, and CI integrity reviewed the completed runtime +chunk. Valid findings corrected guide-selection freezing and exact joins, +CheckerRun-to-Submission Task binding, stale active E2E/docs surfaces, vacuous +legacy assertions, asymmetric immutability proof, and the final schema +fingerprint. All tracks passed after correction; no finding remains open. The +final evidence is in `reviews/WS-XINT-003-02A-internal-review.md`. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md index 83bb7b2b8..e3c710d47 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md @@ -2,8 +2,9 @@ ## Current status -WS-XINT-003-01 contract reconciliation is complete and awaiting human -review/merge. No runtime code or action availability is changed. +WS-XINT-003-01 is merged. WS-XINT-003-02A immutable policy identity and +downstream lineage is implemented on its bounded branch and has passed final +internal review. No policy mutation action or public surface is activated. ## Baseline @@ -28,8 +29,20 @@ REV-owned semantics with AUTH-owned mutation authorization. - All registered review actions remain planned; four lifecycle/recovery actions remain missing until 08R; no service identity is provisioned by chunk 01. +## WS-XINT-003-02A implementation + +- ReviewPolicy and RevisionPolicy are append-only identities with generation, + canonical digest, semantics status, and predecessor lineage. +- ProjectGuide selects exact policy identities; Task locks them, and Submission + and CheckerRun copy and foreign-key chain the same immutable facts. +- Historical rows become readable `legacy_incomplete` records and fail the + canonical readiness predicate. No preference or lease meaning is inferred. +- PostgreSQL rejects update, delete, truncate, cross-project/guide lineage, and + unsafe populated downgrade. +- Focused migration, activation, task, submission/checker, digest, and coverage + proof is green. Repository-wide coverage remains assigned to hosted CI. + ## Next step -Keep hosted exact-head gates green, resolve all external review, and obtain -human merge. Parent 02 plan review then required 02A immutable identity/lineage -before 02B runtime policy mutation activation. +Open the 02A PR, run exact-head hosted CI and CodeRabbit, obtain human merge, +and stop. WS-XINT-003-02B requires a new explicit user start after 02A merges. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md index b3d230818..d1440fe5f 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md @@ -41,7 +41,16 @@ backend/tests/test_checkers.py backend/tests/test_alembic.py backend/tests/test_artifact_admission.py backend/tests/test_policy_identity_lineage.py +backend/tests/conftest.py +backend/scripts/api_contract_e2e.py +backend/scripts/week2_api_e2e.py +examples/terminal_benchmark/terminal_benchmark_api_e2e.py docs/spec_review_lifecycle.md +docs/template_project_guide.md +docs/architecture_data_model.md +docs/glossary.md +docs/spec_chunk_5_submission_packet_foundation.md +docs/spec_chunk_6_checker_contract_records.md .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md .agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/CHUNK_MAP.md diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md new file mode 100644 index 000000000..0c0c3385c --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md @@ -0,0 +1,44 @@ +# Internal Review: WS-XINT-003-02A + +## Scope + +Final working-tree review of immutable review/revision policy identity and its +exact ProjectGuide, Task, Submission, and CheckerRun lineage. + +## Results + +- Architecture: PASS after guide selections became all-or-none, activated + selections became immutable, and repository joins bound the complete exact + identity tuple. +- Security/auth: PASS after the database began binding every CheckerRun to the + same Task as its Submission through `(submission_id, task_id, version)`. +- Product/operations: PASS after active API drills and examples moved to the + complete policy semantics and exact identity triples. +- QA/test: PASS after migration and runtime proof covered guide selection, + downstream lineage, immutability, historical backfill, and fail-closed + readiness. +- Senior engineering: PASS WITH LOW RISKS; formatting-only service churn and + migration size remain review observations, not correctness blockers. +- Reuse/dedup: PASS WITH LOW RISKS; the shared policy-lineage helper is used + rather than duplicating digest/readiness logic. +- Docs: PASS after the guide template, data model, glossary, and submission and + checker specs were updated. +- Test delta: PASS after vacuous legacy assertions were replaced with exact + selected-policy, copied-lineage, redaction, mismatch, and symmetric + immutability assertions. +- CI integrity: PASS after regenerating the exact final schema fingerprint. + +No blocking finding remains. All reviewer sessions completed. + +## Deterministic evidence + +- Ruff passed for application, tests, scripts, and the terminal benchmark. +- The focused migration/lineage selection passed: 10 tests, 74 deselected. +- Policy-lineage branch coverage passed: 9 tests, 100 percent coverage. +- The direct isolated cross-Task CheckerRun mismatch test passed. +- Migration and active E2E Python modules compile. +- Authorization, artifact, wording, review-contract, Markdown-link, and + whitespace checks passed. + +The repository-wide 78-percent coverage suite remains assigned to hosted +GitHub Actions on the exact PR head. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md new file mode 100644 index 000000000..c8a67705b --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md @@ -0,0 +1,67 @@ +# PR Trust Bundle: WS-XINT-003-02A + +## Chunk + +`WS-XINT-003-02A` — Immutable Policy Identity And Lineage. + +## Goal and result + +ReviewPolicy and RevisionPolicy are now immutable, append-only identities. +ProjectGuide selects exact versions, Task locks those exact facts, and +Submission and CheckerRun copy and database-chain them without treating guide +version as policy version. Both policy mutation actions remain planned and +unavailable. + +## Design + +- Complete policies carry typed semantics, generation, canonical digest, + readiness status, and predecessor lineage. +- Historical policies migrate deterministically as readable + `legacy_incomplete` records; missing lease or preference meaning is never + invented and readiness denies their future activation. +- Draft guide selections are all-or-none; activation requires complete + selections and freezes them. +- Exact project, guide, policy ID, generation, and digest tuples are enforced + through downstream foreign keys. +- PostgreSQL rejects policy update, delete, truncate, cross-resource lineage, + active-selection mutation, and populated downgrade. +- No compatibility aliases, route, PREP consumer, grant, or ActionId + activation was added. + +## Proof + +- Focused migration and lineage tests: 10 passed, 74 deselected. +- Policy-lineage branch coverage: 9 passed, 100 percent. +- Direct isolated cross-Task CheckerRun mismatch: 1 passed. +- Ruff, Python compilation, stale contract/wording scans, Markdown links, and + `git diff --check`: passed. +- Hosted GitHub Actions retains the full-suite 78-percent gate and the existing + targeted 90-percent subsystem gate; no threshold or failure behavior changed. + +## Review + +Architecture, security/auth, product/operations, QA/test, docs, test-delta, and +CI integrity passed. Senior engineering and reuse/dedup passed with only low, +non-blocking review observations. Every blocking first-round finding was fixed +and re-reviewed. + +## External review + +GitHub Actions and CodeRabbit must review the exact final PR head. Every valid +comment or failing check must be resolved before human merge. + +## Remaining risk and follow-up + +This chunk establishes identity and lineage but deliberately activates no +policy writer. WS-XINT-003-02B installs the sole authorized mutation path only +after 02A merges and the user explicitly starts that next chunk. + +## Human review focus + +Confirm deterministic legacy handling, immutable exact identity selection, +complete downstream FK chaining, absence of invented semantics, and zero +runtime policy-action activation. + +## Human merge ownership + +Only the human may merge this PR. diff --git a/backend/alembic/versions/0046_immutable_review_revision_policy_lineage.py b/backend/alembic/versions/0046_immutable_review_revision_policy_lineage.py new file mode 100644 index 000000000..afe4367a0 --- /dev/null +++ b/backend/alembic/versions/0046_immutable_review_revision_policy_lineage.py @@ -0,0 +1,597 @@ +"""install immutable review and revision policy identity lineage + +Revision ID: 0046_policy_identity_lineage +Revises: 0045_guide_metadata_authority +Create Date: 2026-08-01 +""" + +from __future__ import annotations + +import hashlib +import json + +from alembic import op +import sqlalchemy as sa + + +revision = "0046_policy_identity_lineage" +down_revision = "0045_guide_metadata_authority" +branch_labels = depends_on = None + + +def _digest(domain: str, value: dict) -> str: + payload = json.dumps( + {"domain": domain, "semantics": value}, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ).encode() + return f"sha256:{hashlib.sha256(payload).hexdigest()}" + + +def _policy_columns() -> None: + for table in ("review_policies", "revision_policies"): + op.add_column(table, sa.Column("policy_generation", sa.Integer())) + op.add_column(table, sa.Column("policy_hash", sa.String(71))) + op.add_column(table, sa.Column("semantics_status", sa.String(24))) + op.add_column(table, sa.Column("supersedes_policy_id", sa.String(36))) + op.create_foreign_key( + f"fk_{table}_supersedes", + table, + table, + ["supersedes_policy_id"], + ["id"], + ) + for name, type_ in ( + ("review_preference_window_seconds", sa.Integer()), + ("review_lease_duration_seconds", sa.Integer()), + ("max_active_review_leases_per_reviewer", sa.Integer()), + ("self_review_allowed", sa.Boolean()), + ("reject_policy", sa.String(32)), + ("finding_evidence_requirement", sa.String(32)), + ): + op.add_column("review_policies", sa.Column(name, type_)) + + +def _backfill_policy_hashes() -> None: + bind = op.get_bind() + review_rows = bind.execute( + sa.text( + "select id,requires_second_review,allowed_decisions,minimum_finding_fields,sla_hours " + "from review_policies" + ) + ).mappings() + for row in review_rows: + value = { + "requires_second_review": row["requires_second_review"], + "allowed_decisions": row["allowed_decisions"], + "minimum_finding_fields": row["minimum_finding_fields"], + "legacy_sla_hours": row["sla_hours"], + } + bind.execute( + sa.text( + "update review_policies set policy_generation=1,policy_hash=:digest," + "semantics_status='legacy_incomplete' where id=:id" + ), + {"id": row["id"], "digest": _digest("workstream.review_policy.legacy.v1", value)}, + ) + revision_rows = bind.execute( + sa.text( + "select id,max_revision_rounds,revision_deadline_hours,auto_reject_after_limit," + "allowed_resubmission_states,reviewer_reassignment_rule from revision_policies" + ) + ).mappings() + for row in revision_rows: + value = { + "max_revision_rounds": row["max_revision_rounds"], + "revision_deadline_hours": row["revision_deadline_hours"], + "legacy_auto_reject_after_limit": row["auto_reject_after_limit"], + "allowed_resubmission_states": row["allowed_resubmission_states"], + "reviewer_reassignment_rule": row["reviewer_reassignment_rule"], + } + bind.execute( + sa.text( + "update revision_policies set policy_generation=1,policy_hash=:digest," + "semantics_status='legacy_incomplete' where id=:id" + ), + {"id": row["id"], "digest": _digest("workstream.revision_policy.legacy.v1", value)}, + ) + + +def _add_lock_columns(table: str, nullable: bool) -> None: + for kind in ("review", "revision"): + op.add_column( + table, sa.Column(f"locked_{kind}_policy_id", sa.String(36), nullable=nullable) + ) + op.add_column( + table, sa.Column(f"locked_{kind}_policy_generation", sa.Integer(), nullable=nullable) + ) + op.add_column( + table, sa.Column(f"locked_{kind}_policy_hash", sa.String(71), nullable=nullable) + ) + + +def _create_immutable_guard(table: str) -> None: + op.execute( + f""" + create function guard_{table}_immutable() returns trigger language plpgsql as $$ + begin + raise exception '{table} rows are immutable' using errcode='55000'; + end $$ + """ + ) + op.execute( + f"create trigger {table}_immutable before update or delete on {table} " + f"for each row execute function guard_{table}_immutable()" + ) + op.execute( + f"create trigger {table}_reject_truncate before truncate on {table} " + f"execute function guard_{table}_immutable()" + ) + + +def upgrade() -> None: + """Move every durable lock to immutable policy identity.""" + _policy_columns() + _backfill_policy_hashes() + for table in ("review_policies", "revision_policies"): + op.alter_column(table, "policy_generation", nullable=False) + op.alter_column(table, "policy_hash", nullable=False) + op.alter_column(table, "semantics_status", nullable=False) + + for name in ( + "selected_review_policy_id", + "selected_review_policy_hash", + "selected_revision_policy_id", + "selected_revision_policy_hash", + ): + op.add_column( + "project_guides", sa.Column(name, sa.String(71 if name.endswith("hash") else 36)) + ) + op.add_column("project_guides", sa.Column("selected_review_policy_generation", sa.Integer())) + op.add_column("project_guides", sa.Column("selected_revision_policy_generation", sa.Integer())) + op.execute( + """ + update project_guides g set + selected_review_policy_id=r.id, + selected_review_policy_generation=r.policy_generation, + selected_review_policy_hash=r.policy_hash, + selected_revision_policy_id=v.id, + selected_revision_policy_generation=v.policy_generation, + selected_revision_policy_hash=v.policy_hash + from review_policies r, revision_policies v + where r.project_id=g.project_id and r.guide_version=g.version + and v.project_id=g.project_id and v.guide_version=g.version + """ + ) + op.execute("set constraints all immediate") + op.create_check_constraint( + "policy_selection_shape", + "project_guides", + "(selected_review_policy_id is null and selected_review_policy_generation is null " + "and selected_review_policy_hash is null and selected_revision_policy_id is null " + "and selected_revision_policy_generation is null and " + "selected_revision_policy_hash is null) or " + "(selected_review_policy_id is not null and " + "selected_review_policy_generation is not null and " + "selected_review_policy_hash is not null and selected_revision_policy_id is not null " + "and selected_revision_policy_generation is not null and " + "selected_revision_policy_hash is not null)", + ) + op.create_check_constraint( + "active_policy_selection_required", + "project_guides", + "status not in ('active','superseded') or " + "(selected_review_policy_id is not null and " + "selected_review_policy_generation is not null and " + "selected_review_policy_hash is not null and selected_revision_policy_id is not null " + "and selected_revision_policy_generation is not null and " + "selected_revision_policy_hash is not null)", + ) + op.execute( + """ + create function guard_project_guide_policy_selection() returns trigger language plpgsql + as $$ begin + if old.status in ('active','superseded') and ( + new.selected_review_policy_id is distinct from old.selected_review_policy_id or + new.selected_review_policy_generation is distinct from + old.selected_review_policy_generation or + new.selected_review_policy_hash is distinct from old.selected_review_policy_hash or + new.selected_revision_policy_id is distinct from old.selected_revision_policy_id or + new.selected_revision_policy_generation is distinct from + old.selected_revision_policy_generation or + new.selected_revision_policy_hash is distinct from old.selected_revision_policy_hash + ) then + raise exception 'active guide policy selection is immutable' using errcode='55000'; + end if; + return new; + end $$ + """ + ) + op.execute( + "create trigger project_guides_policy_selection_immutable before update on " + "project_guides for each row execute function guard_project_guide_policy_selection()" + ) + + _add_lock_columns("workstream_tasks", True) + _add_lock_columns("submissions", True) + _add_lock_columns("checker_runs", True) + op.drop_constraint( + "fk_checker_runs_submission_version", "checker_runs", type_="foreignkey" + ) + op.create_unique_constraint( + "uq_submissions_id_task_version", "submissions", ["id", "task_id", "version"] + ) + op.create_foreign_key( + "fk_checker_runs_submission_version", + "checker_runs", + "submissions", + ["submission_id", "task_id", "submission_version"], + ["id", "task_id", "version"], + ) + op.execute( + """ + update workstream_tasks t set + locked_review_policy_id=r.id, + locked_review_policy_generation=r.policy_generation, + locked_review_policy_hash=r.policy_hash, + locked_revision_policy_id=v.id, + locked_revision_policy_generation=v.policy_generation, + locked_revision_policy_hash=v.policy_hash + from review_policies r, revision_policies v + where r.project_id=t.project_id and r.guide_version=t.locked_review_policy_version + and v.project_id=t.project_id and v.guide_version=t.locked_revision_policy_version + """ + ) + op.create_check_constraint( + "review_revision_policy_lock_shape", + "workstream_tasks", + "(locked_review_policy_id is null and locked_review_policy_generation is null " + "and locked_review_policy_hash is null and locked_revision_policy_id is null " + "and locked_revision_policy_generation is null and locked_revision_policy_hash is null) " + "or (locked_review_policy_id is not null and " + "locked_review_policy_generation is not null and locked_review_policy_hash is not null " + "and locked_revision_policy_id is not null and " + "locked_revision_policy_generation is not null and locked_revision_policy_hash is not null)", + ) + op.create_check_constraint( + "review_revision_policy_lock_required", + "workstream_tasks", + "status='draft' or (locked_review_policy_id is not null and " + "locked_review_policy_generation is not null and locked_review_policy_hash is not null " + "and locked_revision_policy_id is not null and " + "locked_revision_policy_generation is not null and locked_revision_policy_hash is not null)", + ) + for table in ("submissions", "checker_runs"): + op.execute( + f""" + update {table} x set + locked_review_policy_id=t.locked_review_policy_id, + locked_review_policy_generation=t.locked_review_policy_generation, + locked_review_policy_hash=t.locked_review_policy_hash, + locked_revision_policy_id=t.locked_revision_policy_id, + locked_revision_policy_generation=t.locked_revision_policy_generation, + locked_revision_policy_hash=t.locked_revision_policy_hash + from workstream_tasks t where t.id=x.task_id + """ + ) + for kind in ("review", "revision"): + op.alter_column(table, f"locked_{kind}_policy_id", nullable=False) + op.alter_column(table, f"locked_{kind}_policy_generation", nullable=False) + op.alter_column(table, f"locked_{kind}_policy_hash", nullable=False) + + for table, prefix in ( + ("submissions", "submissions_task"), + ("checker_runs", "checker_runs_task"), + ): + op.drop_constraint(f"fk_{prefix}_locked_review_policy", table, type_="foreignkey") + op.drop_constraint(f"fk_{prefix}_locked_revision_policy", table, type_="foreignkey") + op.drop_constraint( + "fk_workstream_tasks_locked_review_policy", "workstream_tasks", type_="foreignkey" + ) + op.drop_constraint( + "fk_workstream_tasks_locked_revision_policy", "workstream_tasks", type_="foreignkey" + ) + op.drop_constraint( + "uq_workstream_tasks_id_locked_review_policy", "workstream_tasks", type_="unique" + ) + op.drop_constraint( + "uq_workstream_tasks_id_locked_revision_policy", "workstream_tasks", type_="unique" + ) + for table in ("workstream_tasks", "submissions", "checker_runs"): + op.drop_column(table, "locked_review_policy_version") + op.drop_column(table, "locked_revision_policy_version") + + op.drop_constraint("uq_review_policies_project_version", "review_policies", type_="unique") + op.drop_constraint("uq_revision_policies_project_version", "revision_policies", type_="unique") + op.create_unique_constraint( + "uq_review_policies_project_version_generation", + "review_policies", + ["project_id", "guide_version", "policy_generation"], + ) + op.create_unique_constraint( + "uq_revision_policies_project_version_generation", + "revision_policies", + ["project_id", "guide_version", "policy_generation"], + ) + for table, kind in (("review_policies", "review"), ("revision_policies", "revision")): + op.create_unique_constraint( + f"uq_{kind}_policy_lineage", table, ["id", "policy_generation", "policy_hash"] + ) + op.create_unique_constraint( + f"uq_{kind}_policy_scoped_lineage", + table, + ["project_id", "guide_version", "id", "policy_generation", "policy_hash"], + ) + op.create_check_constraint( + f"{kind}_policy_identity_shape", + table, + "policy_generation > 0 and policy_hash ~ '^sha256:[0-9a-f]{64}$' " + "and semantics_status in ('complete','legacy_incomplete')", + ) + op.create_check_constraint( + "review_policy_semantics_shape", + "review_policies", + "(semantics_status='legacy_incomplete') or " + "(review_preference_window_seconds > 0 and review_lease_duration_seconds > 0 " + "and max_active_review_leases_per_reviewer=1 and self_review_allowed=false " + "and reject_policy='close_task' and finding_evidence_requirement in " + "('optional','required_for_blocking','required_for_all'))", + ) + op.create_check_constraint( + "revision_policy_semantics_shape", + "revision_policies", + "(semantics_status='legacy_incomplete') or " + "(max_revision_rounds > 0 and revision_deadline_hours > 0)", + ) + # The backfill updates tables with deferrable lineage constraints. Force + # their queued checks to run before PostgreSQL is asked to ALTER those + # same tables for the new composite foreign keys. + op.execute("set constraints all immediate") + op.create_foreign_key( + "fk_project_guides_selected_review_policy", + "project_guides", + "review_policies", + [ + "project_id", + "version", + "selected_review_policy_id", + "selected_review_policy_generation", + "selected_review_policy_hash", + ], + ["project_id", "guide_version", "id", "policy_generation", "policy_hash"], + ) + op.create_foreign_key( + "fk_project_guides_selected_revision_policy", + "project_guides", + "revision_policies", + [ + "project_id", + "version", + "selected_revision_policy_id", + "selected_revision_policy_generation", + "selected_revision_policy_hash", + ], + ["project_id", "guide_version", "id", "policy_generation", "policy_hash"], + ) + for kind in ("review", "revision"): + op.create_unique_constraint( + f"uq_workstream_tasks_id_locked_{kind}_policy", + "workstream_tasks", + [ + "id", + f"locked_{kind}_policy_id", + f"locked_{kind}_policy_generation", + f"locked_{kind}_policy_hash", + ], + ) + op.create_foreign_key( + f"fk_workstream_tasks_locked_{kind}_policy", + "workstream_tasks", + f"{kind}_policies", + [ + "project_id", + "locked_guide_version", + f"locked_{kind}_policy_id", + f"locked_{kind}_policy_generation", + f"locked_{kind}_policy_hash", + ], + ["project_id", "guide_version", "id", "policy_generation", "policy_hash"], + ) + for table, prefix in ( + ("submissions", "submissions_task"), + ("checker_runs", "checker_runs_task"), + ): + op.create_foreign_key( + f"fk_{prefix}_locked_{kind}_policy", + table, + "workstream_tasks", + [ + "task_id", + f"locked_{kind}_policy_id", + f"locked_{kind}_policy_generation", + f"locked_{kind}_policy_hash", + ], + [ + "id", + f"locked_{kind}_policy_id", + f"locked_{kind}_policy_generation", + f"locked_{kind}_policy_hash", + ], + ) + op.drop_column("review_policies", "sla_hours") + op.drop_column("revision_policies", "auto_reject_after_limit") + _create_immutable_guard("review_policies") + _create_immutable_guard("revision_policies") + + +def downgrade() -> None: + """Restore the obsolete schema only when no policy meaning can be lost.""" + bind = op.get_bind() + for table in ("review_policies", "revision_policies"): + count = bind.execute(sa.text(f"select count(*) from {table}")).scalar_one() + if count: + raise RuntimeError("cannot downgrade populated immutable policy lineage") + op.execute("drop trigger project_guides_policy_selection_immutable on project_guides") + op.execute("drop function guard_project_guide_policy_selection()") + op.drop_constraint( + "review_revision_policy_lock_required", + "workstream_tasks", + type_="check", + ) + op.drop_constraint( + "review_revision_policy_lock_shape", + "workstream_tasks", + type_="check", + ) + op.drop_constraint( + "active_policy_selection_required", + "project_guides", + type_="check", + ) + op.drop_constraint( + "policy_selection_shape", "project_guides", type_="check" + ) + for table in ("review_policies", "revision_policies"): + op.execute(f"drop trigger {table}_reject_truncate on {table}") + op.execute(f"drop trigger {table}_immutable on {table}") + op.execute(f"drop function guard_{table}_immutable()") + + op.add_column("review_policies", sa.Column("sla_hours", sa.Integer())) + op.add_column( + "revision_policies", + sa.Column( + "auto_reject_after_limit", sa.Boolean(), server_default=sa.true(), nullable=False + ), + ) + op.drop_constraint( + "fk_checker_runs_submission_version", "checker_runs", type_="foreignkey" + ) + op.drop_constraint( + "uq_submissions_id_task_version", "submissions", type_="unique" + ) + op.create_foreign_key( + "fk_checker_runs_submission_version", + "checker_runs", + "submissions", + ["submission_id", "submission_version"], + ["id", "version"], + ) + for kind in ("review", "revision"): + for table, prefix in ( + ("submissions", "submissions_task"), + ("checker_runs", "checker_runs_task"), + ): + op.drop_constraint(f"fk_{prefix}_locked_{kind}_policy", table, type_="foreignkey") + op.drop_constraint( + f"fk_workstream_tasks_locked_{kind}_policy", "workstream_tasks", type_="foreignkey" + ) + op.drop_constraint( + f"uq_workstream_tasks_id_locked_{kind}_policy", "workstream_tasks", type_="unique" + ) + op.drop_constraint( + "fk_project_guides_selected_review_policy", "project_guides", type_="foreignkey" + ) + op.drop_constraint( + "fk_project_guides_selected_revision_policy", "project_guides", type_="foreignkey" + ) + for table in ("workstream_tasks", "submissions", "checker_runs"): + op.add_column(table, sa.Column("locked_review_policy_version", sa.String(50))) + op.add_column(table, sa.Column("locked_revision_policy_version", sa.String(50))) + op.execute( + """update workstream_tasks t set locked_review_policy_version=r.guide_version, + locked_revision_policy_version=v.guide_version from review_policies r, revision_policies v + where r.id=t.locked_review_policy_id and v.id=t.locked_revision_policy_id""" + ) + for table in ("submissions", "checker_runs"): + op.execute( + f"""update {table} x set locked_review_policy_version=t.locked_review_policy_version, + locked_revision_policy_version=t.locked_revision_policy_version + from workstream_tasks t where t.id=x.task_id""" + ) + op.alter_column(table, "locked_review_policy_version", nullable=False) + op.alter_column(table, "locked_revision_policy_version", nullable=False) + for table in ("workstream_tasks", "submissions", "checker_runs"): + for kind in ("review", "revision"): + op.drop_column(table, f"locked_{kind}_policy_hash") + op.drop_column(table, f"locked_{kind}_policy_generation") + op.drop_column(table, f"locked_{kind}_policy_id") + op.create_unique_constraint( + "uq_workstream_tasks_id_locked_review_policy", + "workstream_tasks", + ["id", "locked_review_policy_version"], + ) + op.create_unique_constraint( + "uq_workstream_tasks_id_locked_revision_policy", + "workstream_tasks", + ["id", "locked_revision_policy_version"], + ) + op.create_unique_constraint( + "uq_review_policies_project_version", + "review_policies", + ["project_id", "guide_version"], + ) + op.create_unique_constraint( + "uq_revision_policies_project_version", + "revision_policies", + ["project_id", "guide_version"], + ) + op.create_foreign_key( + "fk_workstream_tasks_locked_review_policy", + "workstream_tasks", + "review_policies", + ["project_id", "locked_review_policy_version"], + ["project_id", "guide_version"], + ) + op.create_foreign_key( + "fk_workstream_tasks_locked_revision_policy", + "workstream_tasks", + "revision_policies", + ["project_id", "locked_revision_policy_version"], + ["project_id", "guide_version"], + ) + for table, prefix in ( + ("submissions", "submissions_task"), + ("checker_runs", "checker_runs_task"), + ): + for kind in ("review", "revision"): + op.create_foreign_key( + f"fk_{prefix}_locked_{kind}_policy", + table, + "workstream_tasks", + ["task_id", f"locked_{kind}_policy_version"], + ["id", f"locked_{kind}_policy_version"], + ) + for kind in ("review", "revision"): + table = f"{kind}_policies" + op.drop_constraint(f"{kind}_policy_semantics_shape", table, type_="check") + op.drop_constraint(f"{kind}_policy_identity_shape", table, type_="check") + op.drop_constraint(f"uq_{kind}_policy_scoped_lineage", table, type_="unique") + op.drop_constraint(f"uq_{kind}_policy_lineage", table, type_="unique") + op.drop_constraint(f"uq_{kind}_policies_project_version_generation", table, type_="unique") + op.drop_constraint(f"fk_{table}_supersedes", table, type_="foreignkey") + for column in ( + "supersedes_policy_id", + "semantics_status", + "policy_hash", + "policy_generation", + ): + op.drop_column(table, column) + for column in ( + "finding_evidence_requirement", + "reject_policy", + "self_review_allowed", + "max_active_review_leases_per_reviewer", + "review_lease_duration_seconds", + "review_preference_window_seconds", + ): + op.drop_column("review_policies", column) + for column in ( + "selected_revision_policy_hash", + "selected_revision_policy_generation", + "selected_revision_policy_id", + "selected_review_policy_hash", + "selected_review_policy_generation", + "selected_review_policy_id", + ): + op.drop_column("project_guides", column) diff --git a/backend/app/modules/checkers/models.py b/backend/app/modules/checkers/models.py index 6876582a0..1fa416470 100644 --- a/backend/app/modules/checkers/models.py +++ b/backend/app/modules/checkers/models.py @@ -40,17 +40,41 @@ class CheckerRun(Base): "locked_post_submit_checker_policy_version", "locked_post_submit_checker_policy_hash", ], - ["checker_policies.id", "checker_policies.guide_version", "checker_policies.policy_hash"], + [ + "checker_policies.id", + "checker_policies.guide_version", + "checker_policies.policy_hash", + ], name="fk_checker_runs_locked_post_submit_policy_hash", ), ForeignKeyConstraint( - ["task_id", "locked_review_policy_version"], - ["workstream_tasks.id", "workstream_tasks.locked_review_policy_version"], + [ + "task_id", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + ], + [ + "workstream_tasks.id", + "workstream_tasks.locked_review_policy_id", + "workstream_tasks.locked_review_policy_generation", + "workstream_tasks.locked_review_policy_hash", + ], name="fk_checker_runs_task_locked_review_policy", ), ForeignKeyConstraint( - ["task_id", "locked_revision_policy_version"], - ["workstream_tasks.id", "workstream_tasks.locked_revision_policy_version"], + [ + "task_id", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", + ], + [ + "workstream_tasks.id", + "workstream_tasks.locked_revision_policy_id", + "workstream_tasks.locked_revision_policy_generation", + "workstream_tasks.locked_revision_policy_hash", + ], name="fk_checker_runs_task_locked_revision_policy", ), ForeignKeyConstraint( @@ -59,8 +83,8 @@ class CheckerRun(Base): name="fk_checker_runs_task_locked_payment_policy", ), ForeignKeyConstraint( - ["submission_id", "submission_version"], - ["submissions.id", "submissions.version"], + ["submission_id", "task_id", "submission_version"], + ["submissions.id", "submissions.task_id", "submissions.version"], name="fk_checker_runs_submission_version", ), ForeignKeyConstraint( @@ -105,8 +129,12 @@ class CheckerRun(Base): ) id: Mapped[str] = mapped_column(String(36), primary_key=True) - task_id: Mapped[str] = mapped_column(ForeignKey("workstream_tasks.id"), nullable=False, index=True) - submission_id: Mapped[str] = mapped_column(ForeignKey("submissions.id"), nullable=False, index=True) + task_id: Mapped[str] = mapped_column( + ForeignKey("workstream_tasks.id"), nullable=False, index=True + ) + submission_id: Mapped[str] = mapped_column( + ForeignKey("submissions.id"), nullable=False, index=True + ) submission_version: Mapped[int] = mapped_column(Integer, nullable=False) trigger_source: Mapped[str] = mapped_column(String(50), nullable=False) status: Mapped[str] = mapped_column(String(30), nullable=False, default="queued", index=True) @@ -137,8 +165,12 @@ class CheckerRun(Base): ) locked_post_submit_checker_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) locked_post_submit_checker_policy_body: Mapped[dict] = mapped_column(JSON, nullable=False) - locked_review_policy_version: Mapped[str] = mapped_column(String(50), nullable=False) - locked_revision_policy_version: Mapped[str] = mapped_column(String(50), nullable=False) + locked_review_policy_id: Mapped[str] = mapped_column(String(36), nullable=False) + locked_review_policy_generation: Mapped[int] = mapped_column(Integer, nullable=False) + locked_review_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) + locked_revision_policy_id: Mapped[str] = mapped_column(String(36), nullable=False) + locked_revision_policy_generation: Mapped[int] = mapped_column(Integer, nullable=False) + locked_revision_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) locked_payment_policy_version: Mapped[str] = mapped_column(String(50), nullable=False) package_hash: Mapped[str] = mapped_column(String(128), nullable=False) artifact_hash_manifest: Mapped[list[dict]] = mapped_column(JSON, nullable=False, default=list) @@ -171,8 +203,12 @@ class CheckerResult(Base): nullable=False, index=True, ) - task_id: Mapped[str] = mapped_column(ForeignKey("workstream_tasks.id"), nullable=False, index=True) - submission_id: Mapped[str] = mapped_column(ForeignKey("submissions.id"), nullable=False, index=True) + task_id: Mapped[str] = mapped_column( + ForeignKey("workstream_tasks.id"), nullable=False, index=True + ) + submission_id: Mapped[str] = mapped_column( + ForeignKey("submissions.id"), nullable=False, index=True + ) checker_name: Mapped[str] = mapped_column(String(100), nullable=False, index=True) status: Mapped[str] = mapped_column(String(30), nullable=False) severity: Mapped[str] = mapped_column(String(30), nullable=False) diff --git a/backend/app/modules/checkers/runner.py b/backend/app/modules/checkers/runner.py index e748ab8d9..2582e99bb 100644 --- a/backend/app/modules/checkers/runner.py +++ b/backend/app/modules/checkers/runner.py @@ -504,9 +504,7 @@ def _evidence_candidate_tokens(evidence_item: dict) -> set[str]: if value is not None: candidates.add(str(value)) return { - normalized - for candidate in candidates - if (normalized := _normalize_policy_token(candidate)) + normalized for candidate in candidates if (normalized := _normalize_policy_token(candidate)) } @@ -708,10 +706,7 @@ def _size_limit_outcome(manifest: list[dict], effective_policy: dict) -> Checker ) maximum_package_size = effective_policy.get("maximum_package_size_bytes") - known_manifest_size = sum( - entry.get("size_bytes") or 0 - for entry in manifest - ) + known_manifest_size = sum(entry.get("size_bytes") or 0 for entry in manifest) if maximum_package_size is not None and known_manifest_size > maximum_package_size: return _fail( "check_submission_packet", @@ -737,7 +732,9 @@ def _packaging_outcome( allowed_formats = packaging.get("allowed_package_formats") or [] if allowed_formats and payload.package_uri: lowered_uri = payload.package_uri.lower() - if not any(lowered_uri.endswith(f".{str(fmt).lower().lstrip('.')}") for fmt in allowed_formats): + if not any( + lowered_uri.endswith(f".{str(fmt).lower().lstrip('.')}") for fmt in allowed_formats + ): return _fail( "check_submission_packet", "Submission package format is not allowed by the locked project policy.", @@ -852,8 +849,12 @@ async def check_policy_context_present(context: CheckerContext) -> CheckerOutcom "locked_post_submit_checker_policy_hash": ( context.submission.locked_post_submit_checker_policy_hash ), - "locked_review_policy_version": context.submission.locked_review_policy_version, - "locked_revision_policy_version": context.submission.locked_revision_policy_version, + "locked_review_policy_id": context.submission.locked_review_policy_id, + "locked_review_policy_generation": context.submission.locked_review_policy_generation, + "locked_review_policy_hash": context.submission.locked_review_policy_hash, + "locked_revision_policy_id": context.submission.locked_revision_policy_id, + "locked_revision_policy_generation": context.submission.locked_revision_policy_generation, + "locked_revision_policy_hash": context.submission.locked_revision_policy_hash, "locked_payment_policy_version": context.submission.locked_payment_policy_version, "locked_guide_source_snapshot_id": context.submission.locked_guide_source_snapshot_id, "locked_guide_source_snapshot_hash": context.submission.locked_guide_source_snapshot_hash, diff --git a/backend/app/modules/checkers/schemas.py b/backend/app/modules/checkers/schemas.py index 89948dc45..6709b26b8 100644 --- a/backend/app/modules/checkers/schemas.py +++ b/backend/app/modules/checkers/schemas.py @@ -123,8 +123,12 @@ class CheckerRunResponse(BaseModel): locked_post_submit_checker_policy_id: str | None = None locked_post_submit_checker_policy_version: str | None = None locked_post_submit_checker_policy_hash: str | None = None - locked_review_policy_version: str | None - locked_revision_policy_version: str | None + locked_review_policy_id: str | None + locked_review_policy_generation: int | None + locked_review_policy_hash: str | None + locked_revision_policy_id: str | None + locked_revision_policy_generation: int | None + locked_revision_policy_hash: str | None locked_payment_policy_version: str | None package_hash: str | None artifact_hash_manifest: list[dict[str, Any]] | None diff --git a/backend/app/modules/checkers/service.py b/backend/app/modules/checkers/service.py index e4a23fa10..60ee62ccb 100644 --- a/backend/app/modules/checkers/service.py +++ b/backend/app/modules/checkers/service.py @@ -267,9 +267,7 @@ async def _load_locked_pre_submit_context( else task.locked_pre_submit_checker_bundle_hash ) guide_version = ( - submission.locked_guide_version - if submission is not None - else task.locked_guide_version + submission.locked_guide_version if submission is not None else task.locked_guide_version ) source_snapshot_id = ( submission.locked_guide_source_snapshot_id @@ -337,8 +335,7 @@ async def _load_locked_pre_submit_context( ): raise CheckerPolicyInvalid("locked project pre-submit checker policy is invalid") if ( - compiled_bundle.get("effective_policy_hash") - != effective_policy_hash + compiled_bundle.get("effective_policy_hash") != effective_policy_hash or canonical_json_hash(compiled_bundle) != pre_submit_checker_bundle_hash ): raise CheckerPolicyInvalid("locked project pre-submit checker policy is invalid") @@ -350,7 +347,9 @@ async def _load_locked_pre_submit_context( compiler_version=pre_submit_checker_policy.compiler_version, ) except PreSubmitCheckerCompilerError as exc: - raise CheckerPolicyInvalid("locked project pre-submit checker policy is invalid") from exc + raise CheckerPolicyInvalid( + "locked project pre-submit checker policy is invalid" + ) from exc checker_names = list(pre_submit_checker_policy.checker_names or []) if checker_names != compiled_checker_names: raise CheckerPolicyInvalid("locked project pre-submit checker projection is invalid") @@ -542,7 +541,9 @@ async def run_submission_checkers( if latest_submission is None or latest_submission.id != submission.id: raise CheckerExecutionBlocked("only latest submission version can be checked") if task.status not in CHECKER_RUN_ALLOWED_TASK_STATUSES: - raise CheckerExecutionBlocked("task must be submitted or in checker gate before checkers run") + raise CheckerExecutionBlocked( + "task must be submitted or in checker gate before checkers run" + ) execution_actor = audit_actor or actor requester_payload = requester_provenance_payload(requester_actor) if requester_actor else {} current_run = await self._checker_repo.get_current_run_for_submission(submission.id) @@ -668,9 +669,8 @@ async def ensure_automatic_pre_review_gate_queued( current_run = await self._checker_repo.get_current_run_for_submission(submission.id) if current_run is not None: - if ( - current_run.status == "queued" - and self._is_automatic_pre_review_gate_run(current_run) + if current_run.status == "queued" and self._is_automatic_pre_review_gate_run( + current_run ): return ( self._run_response_for_actor( @@ -740,8 +740,12 @@ async def ensure_automatic_pre_review_gate_queued( locked_post_submit_checker_policy_body=( submission.locked_post_submit_checker_policy_body ), - locked_review_policy_version=submission.locked_review_policy_version, - locked_revision_policy_version=submission.locked_revision_policy_version, + locked_review_policy_id=submission.locked_review_policy_id, + locked_review_policy_generation=submission.locked_review_policy_generation, + locked_review_policy_hash=submission.locked_review_policy_hash, + locked_revision_policy_id=submission.locked_revision_policy_id, + locked_revision_policy_generation=submission.locked_revision_policy_generation, + locked_revision_policy_hash=submission.locked_revision_policy_hash, locked_payment_policy_version=submission.locked_payment_policy_version, package_hash=submission.package_hash, artifact_hash_manifest=submission.artifact_hash_manifest, @@ -828,9 +832,8 @@ async def run_queued_pre_review_gate( candidate = await self._checker_repo.get_run(checker_run_id) if candidate is None: raise CheckerRunNotFound("checker run not found") - if ( - not candidate.is_current_for_submission - or not self._is_automatic_pre_review_gate_run(candidate) + if not candidate.is_current_for_submission or not self._is_automatic_pre_review_gate_run( + candidate ): raise CheckerExecutionBlocked("checker run is not an automatic pre-review gate") if not await self._claim_queued_pre_review_gate(checker_run_id): @@ -1178,8 +1181,7 @@ def _build_checker_run( routing_recommendation=routing_recommendation, outcome_source=( "auto_checker" - if routing_recommendation - in {ROUTING_NEEDS_REVISION, ROUTING_TASK_SETUP_BLOCKED} + if routing_recommendation in {ROUTING_NEEDS_REVISION, ROUTING_TASK_SETUP_BLOCKED} else "none" ), triggered_by=actor.actor_id, @@ -1202,8 +1204,12 @@ def _build_checker_run( locked_post_submit_checker_policy_body=( submission.locked_post_submit_checker_policy_body ), - locked_review_policy_version=submission.locked_review_policy_version, - locked_revision_policy_version=submission.locked_revision_policy_version, + locked_review_policy_id=submission.locked_review_policy_id, + locked_review_policy_generation=submission.locked_review_policy_generation, + locked_review_policy_hash=submission.locked_review_policy_hash, + locked_revision_policy_id=submission.locked_revision_policy_id, + locked_revision_policy_generation=submission.locked_revision_policy_generation, + locked_revision_policy_hash=submission.locked_revision_policy_hash, locked_payment_policy_version=submission.locked_payment_policy_version, package_hash=submission.package_hash, artifact_hash_manifest=submission.artifact_hash_manifest, @@ -1247,8 +1253,7 @@ async def _requeue_failed_pre_review_gate_if_needed( PRE_REVIEW_GATE_UNKNOWN_CHECKER_FAILURE_CODE, } is_retryable_failure = ( - checker_run.status == "failed" - and checker_run.failure_code in retryable_failure_codes + checker_run.status == "failed" and checker_run.failure_code in retryable_failure_codes ) if not is_retryable_failure or not self._is_automatic_pre_review_gate_run(checker_run): return False @@ -1294,9 +1299,7 @@ async def _replace_stale_running_pre_review_gate( supersedes_checker_run_id=checker_run.id, is_current_for_submission=True, locked_guide_version=checker_run.locked_guide_version, - locked_post_submit_checker_policy_id=( - checker_run.locked_post_submit_checker_policy_id - ), + locked_post_submit_checker_policy_id=(checker_run.locked_post_submit_checker_policy_id), locked_post_submit_checker_policy_version=( checker_run.locked_post_submit_checker_policy_version ), @@ -1306,8 +1309,12 @@ async def _replace_stale_running_pre_review_gate( locked_post_submit_checker_policy_body=( checker_run.locked_post_submit_checker_policy_body ), - locked_review_policy_version=checker_run.locked_review_policy_version, - locked_revision_policy_version=checker_run.locked_revision_policy_version, + locked_review_policy_id=checker_run.locked_review_policy_id, + locked_review_policy_generation=checker_run.locked_review_policy_generation, + locked_review_policy_hash=checker_run.locked_review_policy_hash, + locked_revision_policy_id=checker_run.locked_revision_policy_id, + locked_revision_policy_generation=checker_run.locked_revision_policy_generation, + locked_revision_policy_hash=checker_run.locked_revision_policy_hash, locked_payment_policy_version=checker_run.locked_payment_policy_version, package_hash=checker_run.package_hash, artifact_hash_manifest=checker_run.artifact_hash_manifest, @@ -1687,8 +1694,12 @@ async def _write_gate_audit( "locked_post_submit_checker_policy_hash": ( submission.locked_post_submit_checker_policy_hash ), - "locked_review_policy_version": submission.locked_review_policy_version, - "locked_revision_policy_version": submission.locked_revision_policy_version, + "locked_review_policy_id": submission.locked_review_policy_id, + "locked_review_policy_generation": submission.locked_review_policy_generation, + "locked_review_policy_hash": submission.locked_review_policy_hash, + "locked_revision_policy_id": submission.locked_revision_policy_id, + "locked_revision_policy_generation": submission.locked_revision_policy_generation, + "locked_revision_policy_hash": submission.locked_revision_policy_hash, "locked_payment_policy_version": submission.locked_payment_policy_version, } if event_payload: @@ -1769,8 +1780,12 @@ async def _write_checker_audit( "locked_post_submit_checker_policy_hash": ( submission.locked_post_submit_checker_policy_hash ), - "locked_review_policy_version": submission.locked_review_policy_version, - "locked_revision_policy_version": submission.locked_revision_policy_version, + "locked_review_policy_id": submission.locked_review_policy_id, + "locked_review_policy_generation": submission.locked_review_policy_generation, + "locked_review_policy_hash": submission.locked_review_policy_hash, + "locked_revision_policy_id": submission.locked_revision_policy_id, + "locked_revision_policy_generation": submission.locked_revision_policy_generation, + "locked_revision_policy_hash": submission.locked_revision_policy_hash, "locked_payment_policy_version": submission.locked_payment_policy_version, **requester_payload, }, @@ -1806,8 +1821,7 @@ def _routing_recommendation_for_outcomes(outcomes: list[CheckerOutcome]) -> str: if any(outcome.routing_recommendation == ROUTING_CHECKER_RETRY for outcome in outcomes): return ROUTING_CHECKER_RETRY if any( - outcome.routing_recommendation == ROUTING_TASK_SETUP_BLOCKED - for outcome in outcomes + outcome.routing_recommendation == ROUTING_TASK_SETUP_BLOCKED for outcome in outcomes ): return ROUTING_TASK_SETUP_BLOCKED if any(outcome.blocks_review for outcome in outcomes): @@ -1839,14 +1853,11 @@ def _apply_blocking_policy( metadata = dict(outcome.metadata) if required_warning: metadata["required_checker_warning_escalated"] = True - blocks_review = ( - status == "failed" - and ( - outcome.blocks_review - or outcome.checker_name in context.required_checker_names - or outcome.severity in context.blocking_severities - or severity in context.blocking_severities - ) + blocks_review = status == "failed" and ( + outcome.blocks_review + or outcome.checker_name in context.required_checker_names + or outcome.severity in context.blocking_severities + or severity in context.blocking_severities ) adjusted.append( replace( @@ -1980,11 +1991,23 @@ def _run_response_for_actor( if has_checker_admin_access else None ), - locked_review_policy_version=( - checker_run.locked_review_policy_version if has_checker_admin_access else None + locked_review_policy_id=( + checker_run.locked_review_policy_id if has_checker_admin_access else None + ), + locked_review_policy_generation=( + checker_run.locked_review_policy_generation if has_checker_admin_access else None + ), + locked_review_policy_hash=( + checker_run.locked_review_policy_hash if has_checker_admin_access else None + ), + locked_revision_policy_id=( + checker_run.locked_revision_policy_id if has_checker_admin_access else None + ), + locked_revision_policy_generation=( + checker_run.locked_revision_policy_generation if has_checker_admin_access else None ), - locked_revision_policy_version=( - checker_run.locked_revision_policy_version if has_checker_admin_access else None + locked_revision_policy_hash=( + checker_run.locked_revision_policy_hash if has_checker_admin_access else None ), locked_payment_policy_version=( checker_run.locked_payment_policy_version if has_checker_admin_access else None diff --git a/backend/app/modules/projects/models.py b/backend/app/modules/projects/models.py index 7f795fab3..bd0190254 100644 --- a/backend/app/modules/projects/models.py +++ b/backend/app/modules/projects/models.py @@ -50,9 +50,7 @@ class Project(Base): slug: Mapped[str] = mapped_column(String(120), nullable=False, unique=True, index=True) description: Mapped[str | None] = mapped_column(Text) status: Mapped[str] = mapped_column(String(30), nullable=False, default="draft", index=True) - created_by_actor_profile_id: Mapped[str | None] = mapped_column( - ForeignKey("actor_profiles.id") - ) + created_by_actor_profile_id: Mapped[str | None] = mapped_column(ForeignKey("actor_profiles.id")) created_via_identity_link_id: Mapped[str | None] = mapped_column( ForeignKey("actor_identity_links.id") ) @@ -96,9 +94,7 @@ class ProjectCreateIdempotencyRecord(Base): name="ck_project_create_request_digest", ), CheckConstraint("operation_generation = 1", name="ck_project_create_generation"), - CheckConstraint( - "status in ('pending','committed')", name="ck_project_create_status" - ), + CheckConstraint("status in ('pending','committed')", name="ck_project_create_status"), CheckConstraint( "(status = 'pending' and committed_at is null) or " "(status = 'committed' and committed_at is not null)", @@ -146,9 +142,7 @@ class GuideMutationIdempotencyRecord(Base): name="ck_guide_mutation_resource_context_digest", ), CheckConstraint("operation_generation > 0", name="ck_guide_mutation_generation"), - CheckConstraint( - "status in ('pending','committed')", name="ck_guide_mutation_status" - ), + CheckConstraint("status in ('pending','committed')", name="ck_guide_mutation_status"), CheckConstraint( "(status='pending' and response_json is null and committed_at is null " "and setup_run_id is null) or " @@ -181,6 +175,66 @@ class ProjectGuide(Base): __tablename__ = "project_guides" __table_args__ = ( UniqueConstraint("project_id", "version", name="uq_project_guides_project_version"), + CheckConstraint( + "(selected_review_policy_id is null and " + "selected_review_policy_generation is null and selected_review_policy_hash is null " + "and selected_revision_policy_id is null and " + "selected_revision_policy_generation is null and " + "selected_revision_policy_hash is null) or " + "(selected_review_policy_id is not null and " + "selected_review_policy_generation is not null and " + "selected_review_policy_hash is not null and " + "selected_revision_policy_id is not null and " + "selected_revision_policy_generation is not null and " + "selected_revision_policy_hash is not null)", + name="policy_selection_shape", + ), + CheckConstraint( + "status not in ('active','superseded') or " + "(selected_review_policy_id is not null and " + "selected_review_policy_generation is not null and " + "selected_review_policy_hash is not null and " + "selected_revision_policy_id is not null and " + "selected_revision_policy_generation is not null and " + "selected_revision_policy_hash is not null)", + name="active_policy_selection_required", + ), + ForeignKeyConstraint( + [ + "project_id", + "version", + "selected_review_policy_id", + "selected_review_policy_generation", + "selected_review_policy_hash", + ], + [ + "review_policies.project_id", + "review_policies.guide_version", + "review_policies.id", + "review_policies.policy_generation", + "review_policies.policy_hash", + ], + name="fk_project_guides_selected_review_policy", + use_alter=True, + ), + ForeignKeyConstraint( + [ + "project_id", + "version", + "selected_revision_policy_id", + "selected_revision_policy_generation", + "selected_revision_policy_hash", + ], + [ + "revision_policies.project_id", + "revision_policies.guide_version", + "revision_policies.id", + "revision_policies.policy_generation", + "revision_policies.policy_hash", + ], + name="fk_project_guides_selected_revision_policy", + use_alter=True, + ), Index( "uq_project_guides_one_active_per_project", "project_id", @@ -221,6 +275,12 @@ class ProjectGuide(Base): onupdate=func.now(), ) superseded_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + selected_review_policy_id: Mapped[str | None] = mapped_column(String(36)) + selected_review_policy_generation: Mapped[int | None] = mapped_column(Integer) + selected_review_policy_hash: Mapped[str | None] = mapped_column(String(71)) + selected_revision_policy_id: Mapped[str | None] = mapped_column(String(36)) + selected_revision_policy_generation: Mapped[int | None] = mapped_column(Integer) + selected_revision_policy_hash: Mapped[str | None] = mapped_column(String(71)) project: Mapped[Project] = relationship(back_populates="guides") @@ -306,7 +366,9 @@ class PostSubmitCheckerPolicy(Base): id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) - guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False, index=True) + guide_id: Mapped[str] = mapped_column( + ForeignKey("project_guides.id"), nullable=False, index=True + ) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) source_snapshot_id: Mapped[str] = mapped_column( ForeignKey("guide_source_snapshots.id"), @@ -358,16 +420,39 @@ class ReviewPolicy(Base): ["project_guides.project_id", "project_guides.version"], name="fk_review_policies_project_guide", ), - UniqueConstraint("project_id", "guide_version", name="uq_review_policies_project_version"), + UniqueConstraint( + "project_id", + "guide_version", + "policy_generation", + name="uq_review_policies_project_version_generation", + ), + UniqueConstraint("id", "policy_generation", "policy_hash", name="uq_review_policy_lineage"), + UniqueConstraint( + "project_id", + "guide_version", + "id", + "policy_generation", + "policy_hash", + name="uq_review_policy_scoped_lineage", + ), ) id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) + policy_generation: Mapped[int] = mapped_column(Integer, nullable=False) + policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) + semantics_status: Mapped[str] = mapped_column(String(24), nullable=False) + supersedes_policy_id: Mapped[str | None] = mapped_column(ForeignKey("review_policies.id")) + review_preference_window_seconds: Mapped[int | None] = mapped_column(Integer) + review_lease_duration_seconds: Mapped[int | None] = mapped_column(Integer) + max_active_review_leases_per_reviewer: Mapped[int | None] = mapped_column(Integer) + self_review_allowed: Mapped[bool | None] = mapped_column(Boolean) + reject_policy: Mapped[str | None] = mapped_column(String(32)) + finding_evidence_requirement: Mapped[str | None] = mapped_column(String(32)) requires_second_review: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) allowed_decisions: Mapped[list[str]] = mapped_column(JSON, nullable=False, default=list) minimum_finding_fields: Mapped[list[str]] = mapped_column(JSON, nullable=False, default=list) - sla_hours: Mapped[int | None] = mapped_column(Integer) created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) @@ -384,16 +469,31 @@ class RevisionPolicy(Base): UniqueConstraint( "project_id", "guide_version", - name="uq_revision_policies_project_version", + "policy_generation", + name="uq_revision_policies_project_version_generation", + ), + UniqueConstraint( + "id", "policy_generation", "policy_hash", name="uq_revision_policy_lineage" + ), + UniqueConstraint( + "project_id", + "guide_version", + "id", + "policy_generation", + "policy_hash", + name="uq_revision_policy_scoped_lineage", ), ) id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) + policy_generation: Mapped[int] = mapped_column(Integer, nullable=False) + policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) + semantics_status: Mapped[str] = mapped_column(String(24), nullable=False) + supersedes_policy_id: Mapped[str | None] = mapped_column(ForeignKey("revision_policies.id")) max_revision_rounds: Mapped[int] = mapped_column(Integer, nullable=False) revision_deadline_hours: Mapped[int] = mapped_column(Integer, nullable=False) - auto_reject_after_limit: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) allowed_resubmission_states: Mapped[list[str]] = mapped_column( JSON, nullable=False, @@ -455,16 +555,16 @@ class GuideSourceSnapshot(Base): id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) - guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False, index=True) + guide_id: Mapped[str] = mapped_column( + ForeignKey("project_guides.id"), nullable=False, index=True + ) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) manifest_schema_version: Mapped[str] = mapped_column(String(50), nullable=False) manifest_json: Mapped[dict] = mapped_column(JSON, nullable=False) bundle_hash: Mapped[str] = mapped_column(String(71), nullable=False, index=True) captured_by: Mapped[str] = mapped_column(String(100), nullable=False) creation_generation: Mapped[int | None] = mapped_column(Integer) - created_by_actor_profile_id: Mapped[str | None] = mapped_column( - ForeignKey("actor_profiles.id") - ) + created_by_actor_profile_id: Mapped[str | None] = mapped_column(ForeignKey("actor_profiles.id")) created_via_identity_link_id: Mapped[str | None] = mapped_column( ForeignKey("actor_identity_links.id") ) @@ -477,7 +577,9 @@ class GuideSourceSnapshot(Base): authorization_decision_event_id: Mapped[str | None] = mapped_column( ForeignKey("audit_events.id") ) - captured_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + captured_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) class GuideSourceSnapshotItem(Base): @@ -588,7 +690,9 @@ class ProjectSetupRun(Base): id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) - guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False, index=True) + guide_id: Mapped[str] = mapped_column( + ForeignKey("project_guides.id"), nullable=False, index=True + ) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) source_snapshot_id: Mapped[str] = mapped_column( ForeignKey("guide_source_snapshots.id"), @@ -668,7 +772,9 @@ class GuideSufficiencyReport(Base): id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) - guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False, index=True) + guide_id: Mapped[str] = mapped_column( + ForeignKey("project_guides.id"), nullable=False, index=True + ) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) source_snapshot_id: Mapped[str] = mapped_column( ForeignKey("guide_source_snapshots.id"), @@ -729,7 +835,9 @@ class SubmissionArtifactPolicy(Base): id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) - guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False, index=True) + guide_id: Mapped[str] = mapped_column( + ForeignKey("project_guides.id"), nullable=False, index=True + ) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) source_snapshot_id: Mapped[str] = mapped_column( ForeignKey("guide_source_snapshots.id"), @@ -738,7 +846,9 @@ class SubmissionArtifactPolicy(Base): ) source_snapshot_hash: Mapped[str] = mapped_column(String(71), nullable=False) policy_version: Mapped[str] = mapped_column(String(50), nullable=False) - lifecycle_status: Mapped[str] = mapped_column(String(30), nullable=False, default="draft", index=True) + lifecycle_status: Mapped[str] = mapped_column( + String(30), nullable=False, default="draft", index=True + ) policy_body: Mapped[dict] = mapped_column(JSON, nullable=False) policy_hash: Mapped[str] = mapped_column(String(71), nullable=False, index=True) derivation_source: Mapped[str] = mapped_column(String(100), nullable=False) @@ -795,7 +905,9 @@ class EffectiveProjectSubmissionArtifactPolicy(Base): id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) - guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False, index=True) + guide_id: Mapped[str] = mapped_column( + ForeignKey("project_guides.id"), nullable=False, index=True + ) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) source_snapshot_id: Mapped[str] = mapped_column( ForeignKey("guide_source_snapshots.id"), @@ -803,9 +915,13 @@ class EffectiveProjectSubmissionArtifactPolicy(Base): index=True, ) source_snapshot_hash: Mapped[str] = mapped_column(String(71), nullable=False) - submission_artifact_policy_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) + submission_artifact_policy_id: Mapped[str] = mapped_column( + String(36), nullable=False, index=True + ) submission_artifact_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) - lifecycle_status: Mapped[str] = mapped_column(String(30), nullable=False, default="approved", index=True) + lifecycle_status: Mapped[str] = mapped_column( + String(30), nullable=False, default="approved", index=True + ) merge_algorithm_version: Mapped[str] = mapped_column(String(50), nullable=False) effective_policy: Mapped[dict] = mapped_column(JSON, nullable=False) effective_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False, index=True) @@ -860,7 +976,9 @@ class PreSubmitCheckerPolicy(Base): id: Mapped[str] = mapped_column(String(36), primary_key=True) project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False, index=True) - guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False, index=True) + guide_id: Mapped[str] = mapped_column( + ForeignKey("project_guides.id"), nullable=False, index=True + ) guide_version: Mapped[str] = mapped_column(String(50), nullable=False) source_snapshot_id: Mapped[str] = mapped_column( ForeignKey("guide_source_snapshots.id"), diff --git a/backend/app/modules/projects/policy_lineage.py b/backend/app/modules/projects/policy_lineage.py new file mode 100644 index 000000000..a8338929b --- /dev/null +++ b/backend/app/modules/projects/policy_lineage.py @@ -0,0 +1,70 @@ +"""Canonical immutable review and revision policy lineage helpers.""" + +from __future__ import annotations + +from typing import Any, Literal + +from pydantic import BaseModel, ConfigDict, Field, ValidationError + +from app.core.hashing import canonical_json_hash + + +PolicySemanticsStatus = Literal["complete", "legacy_incomplete"] + + +class ReviewPolicySemantics(BaseModel): + """Complete v0.1 review-policy facts frozen into downstream work.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + review_preference_window_seconds: int = Field(gt=0) + review_lease_duration_seconds: int = Field(gt=0) + max_active_review_leases_per_reviewer: Literal[1] = 1 + self_review_allowed: Literal[False] = False + reject_policy: Literal["close_task"] = "close_task" + finding_evidence_requirement: Literal[ + "optional", "required_for_blocking", "required_for_all" + ] = "optional" + requires_second_review: bool = False + allowed_decisions: tuple[Literal["accept", "needs_revision", "reject"], ...] + minimum_finding_fields: tuple[str, ...] = () + + +class RevisionPolicySemantics(BaseModel): + """Complete v0.1 human-revision policy facts.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + max_revision_rounds: int = Field(gt=0) + revision_deadline_hours: int = Field(gt=0) + allowed_resubmission_states: tuple[Literal["needs_revision"], ...] + reviewer_reassignment_rule: str | None = None + + +def policy_digest(kind: Literal["review", "revision"], semantics: BaseModel) -> str: + """Return the domain-separated digest for one complete policy version.""" + return canonical_json_hash( + { + "domain": f"workstream.{kind}_policy.v1", + "semantics": semantics.model_dump(mode="json"), + } + ) + + +def require_complete_policy( + *, + kind: Literal["review", "revision"], + status: PolicySemanticsStatus, + policy_hash: str | None, + semantic_values: dict[str, Any], +) -> None: + """Fail closed when semantics are incomplete or their digest is not exact.""" + if status != "complete" or policy_hash is None: + raise ValueError("policy semantics are incomplete") + model = ReviewPolicySemantics if kind == "review" else RevisionPolicySemantics + try: + semantics = model.model_validate(semantic_values) + except ValidationError as exc: + raise ValueError("policy semantics are incomplete") from exc + if policy_digest(kind, semantics) != policy_hash: + raise ValueError("policy semantics digest mismatch") diff --git a/backend/app/modules/projects/repository.py b/backend/app/modules/projects/repository.py index f03e2e026..5ef29f521 100644 --- a/backend/app/modules/projects/repository.py +++ b/backend/app/modules/projects/repository.py @@ -5,7 +5,7 @@ from collections.abc import Sequence from typing import Any -from sqlalchemy import func, select +from sqlalchemy import and_, func, select from sqlalchemy.ext.asyncio import AsyncSession from app.modules.projects.models import ( @@ -735,8 +735,7 @@ async def lock_effective_submission_artifact_policy( .where( EffectiveProjectSubmissionArtifactPolicy.project_id == project_id, EffectiveProjectSubmissionArtifactPolicy.guide_version == guide_version, - EffectiveProjectSubmissionArtifactPolicy.source_snapshot_id - == source_snapshot_id, + EffectiveProjectSubmissionArtifactPolicy.source_snapshot_id == source_snapshot_id, EffectiveProjectSubmissionArtifactPolicy.lifecycle_status == "approved", ) .with_for_update() @@ -821,15 +820,24 @@ async def lock_post_submit_checker_policy_for_guide( raise ProjectRepositoryIntegrityError("multiple current post-submit policies found") return rows[0] if rows else None - async def lock_review_policy( - self, project_id: str, guide_version: str - ) -> ReviewPolicy | None: - """Lock the review policy for one guide version.""" + async def lock_review_policy(self, project_id: str, guide_version: str) -> ReviewPolicy | None: + """Lock the exact review-policy version selected by one guide.""" return await self._session.scalar( select(ReviewPolicy) + .join( + ProjectGuide, + and_( + ProjectGuide.project_id == ReviewPolicy.project_id, + ProjectGuide.version == ReviewPolicy.guide_version, + ProjectGuide.selected_review_policy_id == ReviewPolicy.id, + ProjectGuide.selected_review_policy_generation + == ReviewPolicy.policy_generation, + ProjectGuide.selected_review_policy_hash == ReviewPolicy.policy_hash, + ), + ) .where( - ReviewPolicy.project_id == project_id, - ReviewPolicy.guide_version == guide_version, + ProjectGuide.project_id == project_id, + ProjectGuide.version == guide_version, ) .with_for_update() ) @@ -837,12 +845,23 @@ async def lock_review_policy( async def lock_revision_policy( self, project_id: str, guide_version: str ) -> RevisionPolicy | None: - """Lock the revision policy for one guide version.""" + """Lock the exact revision-policy version selected by one guide.""" return await self._session.scalar( select(RevisionPolicy) + .join( + ProjectGuide, + and_( + ProjectGuide.project_id == RevisionPolicy.project_id, + ProjectGuide.version == RevisionPolicy.guide_version, + ProjectGuide.selected_revision_policy_id == RevisionPolicy.id, + ProjectGuide.selected_revision_policy_generation + == RevisionPolicy.policy_generation, + ProjectGuide.selected_revision_policy_hash == RevisionPolicy.policy_hash, + ), + ) .where( - RevisionPolicy.project_id == project_id, - RevisionPolicy.guide_version == guide_version, + ProjectGuide.project_id == project_id, + ProjectGuide.version == guide_version, ) .with_for_update() ) @@ -1047,29 +1066,6 @@ async def lock_post_submit_checker_policy( ) return result.scalar_one_or_none() - async def upsert_review_policy(self, policy: ReviewPolicy) -> ReviewPolicy: - """Create or replace a review policy for one guide version. - - Args: - policy: Review policy model carrying the desired values. - - Returns: - Persisted review policy model. - """ - existing = await self.get_review_policy(policy.project_id, policy.guide_version) - if existing is None: - self._session.add(policy) - await self._session.flush() - await self._session.refresh(policy) - return policy - existing.requires_second_review = policy.requires_second_review - existing.allowed_decisions = policy.allowed_decisions - existing.minimum_finding_fields = policy.minimum_finding_fields - existing.sla_hours = policy.sla_hours - await self._session.flush() - await self._session.refresh(existing) - return existing - async def get_review_policy(self, project_id: str, guide_version: str) -> ReviewPolicy | None: """Load a review policy by project and guide version. @@ -1081,36 +1077,25 @@ async def get_review_policy(self, project_id: str, guide_version: str) -> Review Review policy when found; otherwise ``None``. """ result = await self._session.execute( - select(ReviewPolicy).where( - ReviewPolicy.project_id == project_id, - ReviewPolicy.guide_version == guide_version, + select(ReviewPolicy) + .join( + ProjectGuide, + and_( + ProjectGuide.project_id == ReviewPolicy.project_id, + ProjectGuide.version == ReviewPolicy.guide_version, + ProjectGuide.selected_review_policy_id == ReviewPolicy.id, + ProjectGuide.selected_review_policy_generation + == ReviewPolicy.policy_generation, + ProjectGuide.selected_review_policy_hash == ReviewPolicy.policy_hash, + ), ) + .where(ProjectGuide.project_id == project_id, ProjectGuide.version == guide_version) ) return result.scalar_one_or_none() - async def upsert_revision_policy(self, policy: RevisionPolicy) -> RevisionPolicy: - """Create or replace a revision policy for one guide version. - - Args: - policy: Revision policy model carrying the desired values. - - Returns: - Persisted revision policy model. - """ - existing = await self.get_revision_policy(policy.project_id, policy.guide_version) - if existing is None: - self._session.add(policy) - await self._session.flush() - await self._session.refresh(policy) - return policy - existing.max_revision_rounds = policy.max_revision_rounds - existing.revision_deadline_hours = policy.revision_deadline_hours - existing.auto_reject_after_limit = policy.auto_reject_after_limit - existing.allowed_resubmission_states = policy.allowed_resubmission_states - existing.reviewer_reassignment_rule = policy.reviewer_reassignment_rule - await self._session.flush() - await self._session.refresh(existing) - return existing + async def get_review_policy_by_id(self, policy_id: str) -> ReviewPolicy | None: + """Load one immutable review-policy version by exact identity.""" + return await self._session.get(ReviewPolicy, policy_id) async def get_revision_policy( self, @@ -1127,13 +1112,26 @@ async def get_revision_policy( Revision policy when found; otherwise ``None``. """ result = await self._session.execute( - select(RevisionPolicy).where( - RevisionPolicy.project_id == project_id, - RevisionPolicy.guide_version == guide_version, + select(RevisionPolicy) + .join( + ProjectGuide, + and_( + ProjectGuide.project_id == RevisionPolicy.project_id, + ProjectGuide.version == RevisionPolicy.guide_version, + ProjectGuide.selected_revision_policy_id == RevisionPolicy.id, + ProjectGuide.selected_revision_policy_generation + == RevisionPolicy.policy_generation, + ProjectGuide.selected_revision_policy_hash == RevisionPolicy.policy_hash, + ), ) + .where(ProjectGuide.project_id == project_id, ProjectGuide.version == guide_version) ) return result.scalar_one_or_none() + async def get_revision_policy_by_id(self, policy_id: str) -> RevisionPolicy | None: + """Load one immutable revision-policy version by exact identity.""" + return await self._session.get(RevisionPolicy, policy_id) + async def upsert_payment_policy(self, policy: PaymentPolicy) -> PaymentPolicy: """Create or replace a payment policy for one guide version. diff --git a/backend/app/modules/projects/schemas.py b/backend/app/modules/projects/schemas.py index 8913ccb88..09a6670e5 100644 --- a/backend/app/modules/projects/schemas.py +++ b/backend/app/modules/projects/schemas.py @@ -15,12 +15,19 @@ class ReviewPolicyInput(BaseModel): model_config = ConfigDict(extra="forbid") + review_preference_window_seconds: int = Field(gt=0) + review_lease_duration_seconds: int = Field(gt=0) + max_active_review_leases_per_reviewer: Literal[1] = 1 + self_review_allowed: Literal[False] = False + reject_policy: Literal["close_task"] = "close_task" + finding_evidence_requirement: Literal[ + "optional", "required_for_blocking", "required_for_all" + ] = "optional" requires_second_review: bool = False allowed_decisions: list[Literal["accept", "needs_revision", "reject"]] = Field( default_factory=lambda: ["accept", "needs_revision", "reject"] ) minimum_finding_fields: list[str] = Field(default_factory=list) - sla_hours: int | None = None class RevisionPolicyInput(BaseModel): @@ -30,7 +37,6 @@ class RevisionPolicyInput(BaseModel): max_revision_rounds: int = Field(ge=1) revision_deadline_hours: int = Field(ge=1) - auto_reject_after_limit: bool = True allowed_resubmission_states: list[str] = Field(default_factory=lambda: ["needs_revision"]) reviewer_reassignment_rule: str | None = None @@ -587,10 +593,19 @@ class ReviewPolicyResponse(BaseModel): id: str project_id: str guide_version: str + policy_generation: int + policy_hash: str | None + semantics_status: Literal["complete", "legacy_incomplete"] + supersedes_policy_id: str | None + review_preference_window_seconds: int | None + review_lease_duration_seconds: int | None + max_active_review_leases_per_reviewer: int | None + self_review_allowed: bool | None + reject_policy: str | None + finding_evidence_requirement: str | None requires_second_review: bool allowed_decisions: list[str] minimum_finding_fields: list[str] - sla_hours: int | None created_at: datetime @@ -602,9 +617,12 @@ class RevisionPolicyResponse(BaseModel): id: str project_id: str guide_version: str + policy_generation: int + policy_hash: str | None + semantics_status: Literal["complete", "legacy_incomplete"] + supersedes_policy_id: str | None max_revision_rounds: int revision_deadline_hours: int - auto_reject_after_limit: bool allowed_resubmission_states: list[str] reviewer_reassignment_rule: str | None created_at: datetime diff --git a/backend/app/modules/projects/service.py b/backend/app/modules/projects/service.py index c9ceb9c8f..8cedb53ac 100644 --- a/backend/app/modules/projects/service.py +++ b/backend/app/modules/projects/service.py @@ -53,6 +53,7 @@ ReviewPolicy, SubmissionArtifactPolicy, ) +from app.modules.projects.policy_lineage import require_complete_policy from app.modules.projects.post_submit_policy import ( DEFAULT_DURABLE_CHECKERS, PostSubmitCheckerCompilerError, @@ -88,9 +89,7 @@ ProjectGuideResponse, ProjectResponse, ProjectSetupRunResponse, - RevisionPolicyInput, RevisionPolicyResponse, - ReviewPolicyInput, ReviewPolicyResponse, SubmissionArtifactPolicyInput, SubmissionArtifactPolicyApprove, @@ -102,7 +101,9 @@ logger = logging.getLogger(__name__) -PROJECT_SETUP_PUBLIC_ERROR_SUMMARY = "project setup failed; inspect server logs with the setup run id" +PROJECT_SETUP_PUBLIC_ERROR_SUMMARY = ( + "project setup failed; inspect server logs with the setup run id" +) PROJECT_SETUP_ROLES = {"admin", "project_manager"} ALLOWED_REVIEW_DECISIONS = {"accept", "needs_revision", "reject"} ALLOWED_REVISION_RESUBMISSION_STATES = {"needs_revision"} @@ -168,6 +169,8 @@ def safe_project_setup_error_summary(summary: str | None) -> str: ): return f"unsupported post-submit checker requirements: {', '.join(unsupported_names)}" return PROJECT_SETUP_PUBLIC_ERROR_SUMMARY + + SECRET_ARTIFACT_TOKEN_SETS = [ {"access", "key"}, {"api", "key"}, @@ -196,9 +199,7 @@ def safe_project_setup_error_summary(summary: str | None) -> str: PROJECT_GUIDE_SUFFICIENCY_AGENT_NAME = "ProjectGuideSufficiencyAgent" PROJECT_GUIDE_SUFFICIENCY_AGENT_VERSION = "workstream-sufficiency-agent-v0.1" SUBMISSION_ARTIFACT_POLICY_DERIVATION_AGENT_NAME = "SubmissionArtifactPolicyDerivationAgent" -SUBMISSION_ARTIFACT_POLICY_DERIVATION_AGENT_VERSION = ( - "workstream-policy-derivation-agent-v0.1" -) +SUBMISSION_ARTIFACT_POLICY_DERIVATION_AGENT_VERSION = "workstream-policy-derivation-agent-v0.1" POST_SUBMIT_CHECKER_POLICY_DERIVATION_AGENT_NAME = "PostSubmitCheckerPolicyDerivationAgent" POST_SUBMIT_CHECKER_POLICY_DERIVATION_AGENT_VERSION = ( "workstream-post-submit-policy-derivation-agent-v0.1" @@ -698,14 +699,13 @@ async def run_guide_sufficiency_agent( try: result = await self._project_agent_runtime().analyze_guide_sufficiency(material) except ProjectAgentRuntimeError: - raise AgentRuntimeUnavailable("project guide sufficiency agent is unavailable") from None + raise AgentRuntimeUnavailable( + "project guide sufficiency agent is unavailable" + ) from None payload = GuideSufficiencyReportCreate( source_snapshot_id=material.source_snapshot_id, status=AGENT_SUFFICIENCY_STATUS_TO_REPORT_STATUS[result.status], - findings=[ - finding.model_dump(mode="json") - for finding in result.findings - ], + findings=[finding.model_dump(mode="json") for finding in result.findings], summary=result.summary, ) self._validate_sufficiency_report_payload(payload) @@ -890,8 +890,7 @@ async def run_submission_artifact_policy_derivation_agent( runtime_report = GuideSufficiencyAgentResult( status=REPORT_STATUS_TO_AGENT_SUFFICIENCY_STATUS[sufficiency_report.status], findings=[ - AgentFinding.model_validate(finding) - for finding in sufficiency_report.findings + AgentFinding.model_validate(finding) for finding in sufficiency_report.findings ], summary=sufficiency_report.summary, agent_name=PROJECT_GUIDE_SUFFICIENCY_AGENT_NAME, @@ -904,7 +903,9 @@ async def run_submission_artifact_policy_derivation_agent( runtime_report, ) except ProjectAgentRuntimeError: - raise AgentRuntimeUnavailable("submission artifact policy agent is unavailable") from None + raise AgentRuntimeUnavailable( + "submission artifact policy agent is unavailable" + ) from None try: policy_input = SubmissionArtifactPolicyInput.model_validate(result.policy_body) @@ -1043,29 +1044,23 @@ async def run_post_submit_checker_policy_derivation_agent( raise PolicySetupBlocked( "compiled project pre-submit checker policy is required before post-submit derivation" ) - superseded_policy = ( - await self._repo.get_latest_superseded_post_submit_checker_policy( - project_id, - guide.id, - guide.version, - snapshot.id, - snapshot.bundle_hash, - effective_policy.id, - effective_policy.effective_policy_hash, - pre_submit_checker_policy.id, - pre_submit_checker_policy.compiled_bundle_hash, - ) + superseded_policy = await self._repo.get_latest_superseded_post_submit_checker_policy( + project_id, + guide.id, + guide.version, + snapshot.id, + snapshot.bundle_hash, + effective_policy.id, + effective_policy.effective_policy_hash, + pre_submit_checker_policy.id, + pre_submit_checker_policy.compiled_bundle_hash, ) has_correction_feedback = ( superseded_policy is not None and superseded_policy.supersession_kind == "correction_requested" ) - superseded_policy_id = ( - superseded_policy.id if has_correction_feedback else None - ) - superseded_policy_hash = ( - superseded_policy.policy_hash if has_correction_feedback else None - ) + superseded_policy_id = superseded_policy.id if has_correction_feedback else None + superseded_policy_hash = superseded_policy.policy_hash if has_correction_feedback else None material = await self._guide_source_material(guide, snapshot) context = self._post_submit_derivation_context( @@ -1090,8 +1085,7 @@ async def run_post_submit_checker_policy_derivation_agent( { "checker_name": reason.checker_name, "evidence_refs": [ - self._safe_bounded_summary_value(ref.ref) - for ref in reason.evidence_refs[:10] + self._safe_bounded_summary_value(ref.ref) for ref in reason.evidence_refs[:10] ], } for reason in result.reasons[:100] @@ -1104,18 +1098,14 @@ async def run_post_submit_checker_policy_derivation_agent( ), "reason_code": "unsupported_required_checker", "evidence_refs": [ - self._safe_bounded_summary_value(ref.ref) - for ref in gap.evidence_refs[:10] + self._safe_bounded_summary_value(ref.ref) for ref in gap.evidence_refs[:10] ], } for gap in result.unsupported_required_checks[:50] ] - unsupported_names = sorted( - {gap["requested_checker"] for gap in unsupported_gaps} - ) + unsupported_names = sorted({gap["requested_checker"] for gap in unsupported_gaps}) raise PolicySetupBlocked( - "unsupported post-submit checker requirements: " - + ", ".join(unsupported_names), + "unsupported post-submit checker requirements: " + ", ".join(unsupported_names), details={"unsupported_required_checks": unsupported_gaps}, ) self._raise_for_unknown_post_submit_checkers(result) @@ -1342,7 +1332,9 @@ async def approve_submission_artifact_policy( await self._ensure_snapshot_is_latest(project_id, guide, snapshot) await self.validate_source_snapshot_integrity(snapshot, PolicySetupBlocked) if policy.source_snapshot_hash != snapshot.bundle_hash: - raise PolicySetupBlocked("submission artifact policy is bound to a stale source snapshot") + raise PolicySetupBlocked( + "submission artifact policy is bound to a stale source snapshot" + ) if self._hash_canonical_json(policy.policy_body) != policy.policy_hash: raise PolicySetupBlocked("submission artifact policy body hash mismatch") if policy.derivation_source == AGENT_SUBMISSION_ARTIFACT_POLICY_DERIVATION_SOURCE: @@ -1731,9 +1723,7 @@ def _active_bundle_response_fields( EffectiveProjectSubmissionArtifactPolicyResponse.model_validate(effective_policy) ), "pre_submit_checker_policy": ( - ActiveGuidePreSubmitCheckerPolicyResponse.model_validate( - pre_submit_checker_policy - ) + ActiveGuidePreSubmitCheckerPolicyResponse.model_validate(pre_submit_checker_policy) ), "post_submit_checker_policy": PostSubmitCheckerPolicyResponse.model_validate( post_submit_checker_policy @@ -1962,8 +1952,8 @@ async def update_project_setup_run_status( if output_post_submit_checker_policy_id is not None: setup_run.output_post_submit_checker_policy_id = output_post_submit_checker_policy_id if post_submit_derivation_summary is not None: - setup_run.post_submit_derivation_summary = ( - self._safe_post_submit_derivation_summary(post_submit_derivation_summary) + setup_run.post_submit_derivation_summary = self._safe_post_submit_derivation_summary( + post_submit_derivation_summary ) setup_run.error_code = error_code setup_run.error_summary = ( @@ -2048,9 +2038,7 @@ async def start_post_submit_setup_continuation( "running_post_submit_derivation_agent", "post_submit_setup_blocked", }: - raise PolicySetupConflict( - "project setup run is not ready for post-submit derivation" - ) + raise PolicySetupConflict("project setup run is not ready for post-submit derivation") now = datetime.now(UTC) setup_run.status = "running_post_submit_derivation_agent" setup_run.current_step = "post_submit_checker_policy_derivation" @@ -2132,8 +2120,7 @@ async def _validate_post_submit_continuation_payload( or post_submit_checker_policy.guide_id != setup_run.guide_id or post_submit_checker_policy.guide_version != setup_run.guide_version or post_submit_checker_policy.source_snapshot_id != setup_run.source_snapshot_id - or post_submit_checker_policy.source_snapshot_hash - != setup_run.source_snapshot_hash + or post_submit_checker_policy.source_snapshot_hash != setup_run.source_snapshot_hash or post_submit_checker_policy.effective_policy_id != effective_policy.id or post_submit_checker_policy.effective_policy_hash != effective_policy.effective_policy_hash @@ -2164,8 +2151,7 @@ async def _validate_post_submit_policy_matches_continuation_payload( or post_submit_checker_policy.guide_id != setup_run.guide_id or post_submit_checker_policy.guide_version != setup_run.guide_version or post_submit_checker_policy.source_snapshot_id != setup_run.source_snapshot_id - or post_submit_checker_policy.source_snapshot_hash - != setup_run.source_snapshot_hash + or post_submit_checker_policy.source_snapshot_hash != setup_run.source_snapshot_hash or post_submit_checker_policy.effective_policy_id != effective_policy_id or post_submit_checker_policy.pre_submit_checker_policy_id != pre_submit_checker_policy_id @@ -2201,9 +2187,7 @@ async def _validate_project_setup_run_outputs( setup_run, post_submit_policy, ): - raise PolicySetupConflict( - "project setup run post-submit policy output mismatch" - ) + raise PolicySetupConflict("project setup run post-submit policy output mismatch") async def _post_submit_policy_from_setup_run( self, @@ -2301,7 +2285,9 @@ async def _post_submit_policy_setup_response( guide_version=setup_run.guide_version, setup_run=ProjectSetupRunResponse.model_validate(setup_run), post_submit_checker_policy=policy_summary, - derivation_input_summary=await self._post_submit_derivation_input_summary(setup_run, policy), + derivation_input_summary=await self._post_submit_derivation_input_summary( + setup_run, policy + ), correction_history=await self._post_submit_policy_correction_history(setup_run), ) @@ -2422,8 +2408,10 @@ async def _post_submit_derivation_input_summary( summary["effective_policy_forbidden_artifact_count"] = len( effective_body.get("forbidden_artifacts") or [] ) - pre_submit_policy = await self._repo.get_pre_submit_checker_policy_for_effective_policy( - effective_policy.id + pre_submit_policy = ( + await self._repo.get_pre_submit_checker_policy_for_effective_policy( + effective_policy.id + ) ) if ( pre_submit_policy is not None @@ -2532,10 +2520,7 @@ def _safe_bounded_summary_value(self, value: str) -> str: def _safe_public_unsupported_requirement(self, value: str) -> str: """Return a safe operator-visible label for an unsupported requirement.""" normalized = value.strip().lower() - if ( - not normalized.startswith("check_") - or not SAFE_TOKEN_PATTERN.fullmatch(normalized) - ): + if not normalized.startswith("check_") or not SAFE_TOKEN_PATTERN.fullmatch(normalized): return "unsupported checker requirement" return normalized @@ -2577,9 +2562,7 @@ async def _source_snapshot_response( """Build a snapshot response with ordered source items.""" items = await self._repo.list_guide_source_snapshot_items(snapshot.id) response = GuideSourceSnapshotResponse.model_validate(snapshot) - response.items = [ - GuideSourceSnapshotItemResponse.model_validate(item) for item in items - ] + response.items = [GuideSourceSnapshotItemResponse.model_validate(item) for item in items] return response async def _guide_source_material( @@ -2599,8 +2582,7 @@ async def _guide_source_material( source_snapshot_id=snapshot.id, source_snapshot_hash=snapshot.bundle_hash, guide_material={ - field: getattr(guide, field) - for field in sorted(GUIDE_SOURCE_MATERIAL_FIELDS) + field: getattr(guide, field) for field in sorted(GUIDE_SOURCE_MATERIAL_FIELDS) }, source_items=source_items, source_refs=[item.durable_ref for item in source_items], @@ -2615,9 +2597,7 @@ def _source_material_items( ) -> list[GuideSourceItemMaterial]: """Return typed source items from a guide-source snapshot manifest.""" return [ - GuideSourceItemMaterial.model_validate( - self._normalized_source_manifest_item(item) - ) + GuideSourceItemMaterial.model_validate(self._normalized_source_manifest_item(item)) for item in snapshot.manifest_json["items"] ] @@ -2959,8 +2939,7 @@ def _merge_effective_submission_artifact_policy( default_policy["manifest_required"] or project_policy["manifest_required"] ), "artifact_hash_required": bool( - default_policy["artifact_hash_required"] - or project_policy["artifact_hash_required"] + default_policy["artifact_hash_required"] or project_policy["artifact_hash_required"] ), "artifact_hash_algorithm": PLATFORM_HASH_ALGORITHM, "allowed_storage_schemes": allowed_storage_schemes, @@ -3117,11 +3096,7 @@ def _contains_secret_artifact_tokens(self, value: str) -> bool: """Return whether any path segment uses credential-like words.""" all_tokens: set[str] = set() for segment in value.split("/"): - tokens = { - token - for token in re.split(r"[^a-z0-9]+", segment.lower()) - if token - } + tokens = {token for token in re.split(r"[^a-z0-9]+", segment.lower()) if token} all_tokens.update(tokens) if tokens.intersection(SECRET_ARTIFACT_SINGLE_TOKENS): return True @@ -3174,7 +3149,9 @@ def _validate_sufficiency_report_allows_policy_derivation( ) -> None: """Require report freshness and no blocking gaps before deriving policy.""" if sufficiency_report is None: - raise PolicySetupBlocked("guide sufficiency report is required before policy derivation") + raise PolicySetupBlocked( + "guide sufficiency report is required before policy derivation" + ) if sufficiency_report.source_snapshot_id != source_snapshot.id: raise PolicySetupBlocked("guide sufficiency report is bound to a stale snapshot") if sufficiency_report.source_snapshot_hash != source_snapshot.bundle_hash: @@ -3224,15 +3201,14 @@ def _validate_agent_derived_submission_artifact_policy( ) if ( policy.derivation_agent_name != SUBMISSION_ARTIFACT_POLICY_DERIVATION_AGENT_NAME - or policy.derivation_agent_version != SUBMISSION_ARTIFACT_POLICY_DERIVATION_AGENT_VERSION + or policy.derivation_agent_version + != SUBMISSION_ARTIFACT_POLICY_DERIVATION_AGENT_VERSION ): raise PolicySetupConflict( "agent-derived submission artifact policy runtime provenance is not server-owned" ) if self._hash_canonical_json(policy.policy_body) != policy.policy_hash: - raise PolicySetupConflict( - "agent-derived submission artifact policy body hash mismatch" - ) + raise PolicySetupConflict("agent-derived submission artifact policy body hash mismatch") def _post_submit_derivation_context( self, @@ -3366,9 +3342,7 @@ def _raise_for_unknown_post_submit_checkers( reason = reason_by_checker.get(checker_name) unsupported_gaps.append( { - "requested_checker": self._safe_public_unsupported_requirement( - checker_name - ), + "requested_checker": self._safe_public_unsupported_requirement(checker_name), "reason_code": "unsupported_required_checker", "evidence_refs": [ self._safe_bounded_summary_value(ref.ref) @@ -3376,12 +3350,9 @@ def _raise_for_unknown_post_submit_checkers( ], } ) - unsupported_names = sorted( - {gap["requested_checker"] for gap in unsupported_gaps} - ) + unsupported_names = sorted({gap["requested_checker"] for gap in unsupported_gaps}) raise PolicySetupBlocked( - "unsupported post-submit checker requirements: " - + ", ".join(unsupported_names), + "unsupported post-submit checker requirements: " + ", ".join(unsupported_names), details={"unsupported_required_checks": unsupported_gaps}, ) @@ -3461,7 +3432,10 @@ def validate_activation_ready( raise GuideActivationBlocked("submission artifact policy is bound to a stale snapshot") if submission_artifact_policy.source_snapshot_hash != source_snapshot.bundle_hash: raise GuideActivationBlocked("submission artifact policy snapshot hash mismatch") - if submission_artifact_policy.derivation_source == AGENT_SUBMISSION_ARTIFACT_POLICY_DERIVATION_SOURCE: + if ( + submission_artifact_policy.derivation_source + == AGENT_SUBMISSION_ARTIFACT_POLICY_DERIVATION_SOURCE + ): try: self._validate_agent_derived_submission_artifact_policy( submission_artifact_policy, @@ -3474,14 +3448,21 @@ def validate_activation_ready( != submission_artifact_policy.policy_hash ): raise GuideActivationBlocked("submission artifact policy body hash mismatch") - if not submission_artifact_policy.approved_by_actor or not submission_artifact_policy.approved_at: - raise GuideActivationBlocked("submission artifact policy approval provenance is required") + if ( + not submission_artifact_policy.approved_by_actor + or not submission_artifact_policy.approved_at + ): + raise GuideActivationBlocked( + "submission artifact policy approval provenance is required" + ) if submission_artifact_policy.approved_by_role not in PROJECT_SETUP_ROLES: raise GuideActivationBlocked("submission artifact policy approver role is invalid") if effective_policy is None: raise GuideActivationBlocked("effective project submission artifact policy is required") if effective_policy.lifecycle_status != "approved": - raise GuideActivationBlocked("effective project submission artifact policy is not approved") + raise GuideActivationBlocked( + "effective project submission artifact policy is not approved" + ) if effective_policy.source_snapshot_id != source_snapshot.id: raise GuideActivationBlocked( "effective project submission artifact policy is bound to a stale snapshot" @@ -3502,10 +3483,11 @@ def validate_activation_ready( submission_artifact_policy.policy_body ) except (KeyError, TypeError, ValueError, ProjectServiceError) as exc: - raise GuideActivationBlocked( - "submission artifact policy body is invalid" - ) from exc - if self._hash_canonical_json(expected_effective_policy) != effective_policy.effective_policy_hash: + raise GuideActivationBlocked("submission artifact policy body is invalid") from exc + if ( + self._hash_canonical_json(expected_effective_policy) + != effective_policy.effective_policy_hash + ): raise GuideActivationBlocked( "effective project submission artifact policy no longer matches submission policy" ) @@ -3513,7 +3495,10 @@ def validate_activation_ready( raise GuideActivationBlocked( "effective project submission artifact policy is bound to the wrong policy" ) - if effective_policy.submission_artifact_policy_hash != submission_artifact_policy.policy_hash: + if ( + effective_policy.submission_artifact_policy_hash + != submission_artifact_policy.policy_hash + ): raise GuideActivationBlocked( "effective project submission artifact policy hash provenance mismatch" ) @@ -3527,7 +3512,10 @@ def validate_activation_ready( raise GuideActivationBlocked( "pre-submit checker policy is bound to the wrong effective policy" ) - if pre_submit_checker_policy.effective_policy_hash != effective_policy.effective_policy_hash: + if ( + pre_submit_checker_policy.effective_policy_hash + != effective_policy.effective_policy_hash + ): raise GuideActivationBlocked("pre-submit checker bundle provenance mismatch") if pre_submit_checker_policy.lifecycle_status != "compiled": raise GuideActivationBlocked("compiled project pre-submit checker policy is required") @@ -3558,7 +3546,10 @@ def validate_activation_ready( raise GuideActivationBlocked( "post-submit checker policy is bound to the wrong effective policy" ) - if post_submit_checker_policy.effective_policy_hash != effective_policy.effective_policy_hash: + if ( + post_submit_checker_policy.effective_policy_hash + != effective_policy.effective_policy_hash + ): raise GuideActivationBlocked("post-submit checker policy effective hash mismatch") if post_submit_checker_policy.pre_submit_checker_policy_id != pre_submit_checker_policy.id: raise GuideActivationBlocked( @@ -3606,10 +3597,46 @@ def validate_activation_ready( ) from exc if review_policy is None or not review_policy.allowed_decisions: raise GuideActivationBlocked("review policy with allowed decisions is required") - if not set(review_policy.allowed_decisions).issubset(ALLOWED_REVIEW_DECISIONS): - raise GuideActivationBlocked("review policy contains invalid decisions") if revision_policy is None: raise GuideActivationBlocked("revision policy is required") + try: + require_complete_policy( + kind="review", + status=review_policy.semantics_status, + policy_hash=review_policy.policy_hash, + semantic_values={ + "review_preference_window_seconds": ( + review_policy.review_preference_window_seconds + ), + "review_lease_duration_seconds": review_policy.review_lease_duration_seconds, + "max_active_review_leases_per_reviewer": ( + review_policy.max_active_review_leases_per_reviewer + ), + "self_review_allowed": review_policy.self_review_allowed, + "reject_policy": review_policy.reject_policy, + "finding_evidence_requirement": review_policy.finding_evidence_requirement, + "requires_second_review": review_policy.requires_second_review, + "allowed_decisions": review_policy.allowed_decisions, + "minimum_finding_fields": review_policy.minimum_finding_fields, + }, + ) + require_complete_policy( + kind="revision", + status=revision_policy.semantics_status, + policy_hash=revision_policy.policy_hash, + semantic_values={ + "max_revision_rounds": revision_policy.max_revision_rounds, + "revision_deadline_hours": revision_policy.revision_deadline_hours, + "allowed_resubmission_states": revision_policy.allowed_resubmission_states, + "reviewer_reassignment_rule": revision_policy.reviewer_reassignment_rule, + }, + ) + except ValueError as exc: + raise GuideActivationBlocked( + "review and revision policy semantics are incomplete" + ) from exc + if not set(review_policy.allowed_decisions).issubset(ALLOWED_REVIEW_DECISIONS): + raise GuideActivationBlocked("review policy contains invalid decisions") if ( revision_policy.max_revision_rounds < 1 or revision_policy.revision_deadline_hours < 1 @@ -3649,59 +3676,6 @@ def _validate_sufficiency_warning_acknowledgement( f"guide sufficiency warnings require admin/project_manager acknowledgement {action}" ) - def _review_policy_model( - self, - project_id: str, - guide_version: str, - payload: ReviewPolicyInput, - ) -> ReviewPolicy: - """Build a review policy model from API input. - - Args: - project_id: Project that owns the policy. - guide_version: Guide version the policy applies to. - payload: Validated review policy input. - - Returns: - Unsaved review policy model. - """ - return ReviewPolicy( - id=str(uuid4()), - project_id=project_id, - guide_version=guide_version, - requires_second_review=payload.requires_second_review, - allowed_decisions=payload.allowed_decisions, - minimum_finding_fields=payload.minimum_finding_fields, - sla_hours=payload.sla_hours, - ) - - def _revision_policy_model( - self, - project_id: str, - guide_version: str, - payload: RevisionPolicyInput, - ) -> RevisionPolicy: - """Build a revision policy model from API input. - - Args: - project_id: Project that owns the policy. - guide_version: Guide version the policy applies to. - payload: Validated revision policy input. - - Returns: - Unsaved revision policy model. - """ - return RevisionPolicy( - id=str(uuid4()), - project_id=project_id, - guide_version=guide_version, - max_revision_rounds=payload.max_revision_rounds, - revision_deadline_hours=payload.revision_deadline_hours, - auto_reject_after_limit=payload.auto_reject_after_limit, - allowed_resubmission_states=payload.allowed_resubmission_states, - reviewer_reassignment_rule=payload.reviewer_reassignment_rule, - ) - def _payment_policy_model( self, project_id: str, @@ -3800,14 +3774,10 @@ def build_guide_source_snapshot_manifest( seen_refs = {("project_guide", normalized_items[0]["durable_ref"])} for item in payload.items: source_kind = _guide_source_token(item.source_kind, "source kind") - ingestion_adapter = _guide_source_token( - item.ingestion_adapter, "ingestion adapter" - ) + ingestion_adapter = _guide_source_token(item.ingestion_adapter, "ingestion adapter") durable_ref = _guide_source_durable_ref(item.durable_ref) if not HASH_PATTERN.fullmatch(item.content_hash): - raise PolicySetupBlocked( - "source item content hash must be sha256:<64 lowercase hex>" - ) + raise PolicySetupBlocked("source item content hash must be sha256:<64 lowercase hex>") content_cid = _guide_source_content_cid(item.content_cid) duplicate_key = (source_kind, durable_ref) if duplicate_key in seen_refs: diff --git a/backend/app/modules/tasks/models.py b/backend/app/modules/tasks/models.py index 6e43f69a8..789ec3494 100644 --- a/backend/app/modules/tasks/models.py +++ b/backend/app/modules/tasks/models.py @@ -32,6 +32,25 @@ class WorkstreamTask(Base): __tablename__ = "workstream_tasks" __table_args__ = ( + CheckConstraint( + "(locked_review_policy_id is null and locked_review_policy_generation is null " + "and locked_review_policy_hash is null and locked_revision_policy_id is null " + "and locked_revision_policy_generation is null and locked_revision_policy_hash is null) " + "or (locked_review_policy_id is not null and " + "locked_review_policy_generation is not null and locked_review_policy_hash is not null " + "and locked_revision_policy_id is not null and " + "locked_revision_policy_generation is not null and " + "locked_revision_policy_hash is not null)", + name="review_revision_policy_lock_shape", + ), + CheckConstraint( + "status = 'draft' or (locked_review_policy_id is not null and " + "locked_review_policy_generation is not null and locked_review_policy_hash is not null " + "and locked_revision_policy_id is not null and " + "locked_revision_policy_generation is not null and " + "locked_revision_policy_hash is not null)", + name="review_revision_policy_lock_required", + ), ForeignKeyConstraint( ["project_id", "locked_guide_version"], ["project_guides.project_id", "project_guides.version"], @@ -43,17 +62,45 @@ class WorkstreamTask(Base): "locked_post_submit_checker_policy_version", "locked_post_submit_checker_policy_hash", ], - ["checker_policies.id", "checker_policies.guide_version", "checker_policies.policy_hash"], + [ + "checker_policies.id", + "checker_policies.guide_version", + "checker_policies.policy_hash", + ], name="fk_workstream_tasks_locked_post_submit_policy_hash", ), ForeignKeyConstraint( - ["project_id", "locked_review_policy_version"], - ["review_policies.project_id", "review_policies.guide_version"], + [ + "project_id", + "locked_guide_version", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + ], + [ + "review_policies.project_id", + "review_policies.guide_version", + "review_policies.id", + "review_policies.policy_generation", + "review_policies.policy_hash", + ], name="fk_workstream_tasks_locked_review_policy", ), ForeignKeyConstraint( - ["project_id", "locked_revision_policy_version"], - ["revision_policies.project_id", "revision_policies.guide_version"], + [ + "project_id", + "locked_guide_version", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", + ], + [ + "revision_policies.project_id", + "revision_policies.guide_version", + "revision_policies.id", + "revision_policies.policy_generation", + "revision_policies.policy_hash", + ], name="fk_workstream_tasks_locked_revision_policy", ), ForeignKeyConstraint( @@ -92,12 +139,16 @@ class WorkstreamTask(Base): ), UniqueConstraint( "id", - "locked_review_policy_version", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", name="uq_workstream_tasks_id_locked_review_policy", ), UniqueConstraint( "id", - "locked_revision_policy_version", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", name="uq_workstream_tasks_id_locked_revision_policy", ), UniqueConstraint( @@ -160,8 +211,12 @@ class WorkstreamTask(Base): locked_post_submit_checker_policy_version: Mapped[str | None] = mapped_column(String(50)) locked_post_submit_checker_policy_hash: Mapped[str | None] = mapped_column(String(71)) locked_post_submit_checker_policy_body: Mapped[dict | None] = mapped_column(JSON) - locked_review_policy_version: Mapped[str | None] = mapped_column(String(50)) - locked_revision_policy_version: Mapped[str | None] = mapped_column(String(50)) + locked_review_policy_id: Mapped[str | None] = mapped_column(String(36)) + locked_review_policy_generation: Mapped[int | None] = mapped_column(Integer) + locked_review_policy_hash: Mapped[str | None] = mapped_column(String(71)) + locked_revision_policy_id: Mapped[str | None] = mapped_column(String(36)) + locked_revision_policy_generation: Mapped[int | None] = mapped_column(Integer) + locked_revision_policy_hash: Mapped[str | None] = mapped_column(String(71)) locked_payment_policy_version: Mapped[str | None] = mapped_column(String(50)) locked_guide_source_snapshot_id: Mapped[str | None] = mapped_column(String(36)) locked_guide_source_snapshot_hash: Mapped[str | None] = mapped_column(String(71)) @@ -239,7 +294,9 @@ class TaskAssignment(Base): index=True, ) assigned_by: Mapped[str] = mapped_column(String(100), nullable=False) - assigned_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + assigned_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) accepted_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) released_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) status: Mapped[str] = mapped_column(String(30), nullable=False, default="active", index=True) @@ -273,13 +330,33 @@ class Submission(Base): name="fk_submissions_task_locked_post_submit_policy_hash", ), ForeignKeyConstraint( - ["task_id", "locked_review_policy_version"], - ["workstream_tasks.id", "workstream_tasks.locked_review_policy_version"], + [ + "task_id", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + ], + [ + "workstream_tasks.id", + "workstream_tasks.locked_review_policy_id", + "workstream_tasks.locked_review_policy_generation", + "workstream_tasks.locked_review_policy_hash", + ], name="fk_submissions_task_locked_review_policy", ), ForeignKeyConstraint( - ["task_id", "locked_revision_policy_version"], - ["workstream_tasks.id", "workstream_tasks.locked_revision_policy_version"], + [ + "task_id", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", + ], + [ + "workstream_tasks.id", + "workstream_tasks.locked_revision_policy_id", + "workstream_tasks.locked_revision_policy_generation", + "workstream_tasks.locked_revision_policy_hash", + ], name="fk_submissions_task_locked_revision_policy", ), ForeignKeyConstraint( @@ -310,7 +387,11 @@ class Submission(Base): name="fk_submissions_task_locked_effective_policy_hash", ), ForeignKeyConstraint( - ["task_id", "locked_pre_submit_checker_policy_id", "locked_pre_submit_checker_bundle_hash"], + [ + "task_id", + "locked_pre_submit_checker_policy_id", + "locked_pre_submit_checker_bundle_hash", + ], [ "workstream_tasks.id", "workstream_tasks.locked_pre_submit_checker_policy_id", @@ -345,11 +426,18 @@ class Submission(Base): "locked_post_submit_checker_policy_version", "locked_post_submit_checker_policy_hash", ], - ["checker_policies.id", "checker_policies.guide_version", "checker_policies.policy_hash"], + [ + "checker_policies.id", + "checker_policies.guide_version", + "checker_policies.policy_hash", + ], name="fk_submissions_locked_post_submit_policy_hash", ), UniqueConstraint("task_id", "version", name="uq_submissions_task_version"), UniqueConstraint("id", "version", name="uq_submissions_id_version"), + UniqueConstraint( + "id", "task_id", "version", name="uq_submissions_id_task_version" + ), UniqueConstraint( "id", "locked_post_submit_checker_policy_id", @@ -385,7 +473,9 @@ class Submission(Base): ) id: Mapped[str] = mapped_column(String(36), primary_key=True) - task_id: Mapped[str] = mapped_column(ForeignKey("workstream_tasks.id"), nullable=False, index=True) + task_id: Mapped[str] = mapped_column( + ForeignKey("workstream_tasks.id"), nullable=False, index=True + ) contributor_id: Mapped[str] = mapped_column( String(36), ForeignKey("actor_profiles.id"), @@ -407,8 +497,12 @@ class Submission(Base): ) locked_post_submit_checker_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) locked_post_submit_checker_policy_body: Mapped[dict] = mapped_column(JSON, nullable=False) - locked_review_policy_version: Mapped[str] = mapped_column(String(50), nullable=False) - locked_revision_policy_version: Mapped[str] = mapped_column(String(50), nullable=False) + locked_review_policy_id: Mapped[str] = mapped_column(String(36), nullable=False) + locked_review_policy_generation: Mapped[int] = mapped_column(Integer, nullable=False) + locked_review_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) + locked_revision_policy_id: Mapped[str] = mapped_column(String(36), nullable=False) + locked_revision_policy_generation: Mapped[int] = mapped_column(Integer, nullable=False) + locked_revision_policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) locked_payment_policy_version: Mapped[str] = mapped_column(String(50), nullable=False) locked_guide_source_snapshot_id: Mapped[str | None] = mapped_column(String(36)) locked_guide_source_snapshot_hash: Mapped[str | None] = mapped_column(String(71)) @@ -422,7 +516,9 @@ class Submission(Base): locked_pre_submit_checker_bundle_hash: Mapped[str | None] = mapped_column( String(71), ) - submitted_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + submitted_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.now() + ) locked_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) supersedes_submission_id: Mapped[str | None] = mapped_column( ForeignKey("submissions.id"), diff --git a/backend/app/modules/tasks/schemas.py b/backend/app/modules/tasks/schemas.py index b6169ab87..df8658988 100644 --- a/backend/app/modules/tasks/schemas.py +++ b/backend/app/modules/tasks/schemas.py @@ -153,8 +153,12 @@ class TaskResponse(BaseModel): id: str project_id: str locked_guide_version: str | None - locked_review_policy_version: str | None - locked_revision_policy_version: str | None + locked_review_policy_id: str | None + locked_review_policy_generation: int | None + locked_review_policy_hash: str | None + locked_revision_policy_id: str | None + locked_revision_policy_generation: int | None + locked_revision_policy_hash: str | None locked_payment_policy_version: str | None locked_guide_source_snapshot_id: str | None locked_guide_source_snapshot_hash: str | None @@ -231,13 +235,17 @@ class TaskGuideContext(BaseModel): class TaskReviewPolicyContext(BaseModel): """Contributor-safe review policy summary for the locked guide version.""" - guide_version: str + policy_id: str + policy_generation: int + policy_hash: str class TaskRevisionPolicyContext(BaseModel): """Contributor-safe revision policy summary for the locked guide version.""" - guide_version: str + policy_id: str + policy_generation: int + policy_hash: str class TaskPaymentPolicyContext(BaseModel): @@ -361,8 +369,12 @@ class TaskLockedContextResponse(BaseModel): locked_post_submit_checker_policy_version: str locked_post_submit_checker_policy_hash: str locked_post_submit_checker_policy_body_summary: PostSubmitPolicyBodySummary - locked_review_policy_version: str - locked_revision_policy_version: str + locked_review_policy_id: str + locked_review_policy_generation: int + locked_review_policy_hash: str + locked_revision_policy_id: str + locked_revision_policy_generation: int + locked_revision_policy_hash: str locked_payment_policy_version: str @@ -429,8 +441,12 @@ class SubmissionResponse(BaseModel): artifact_hash_manifest: list[dict[str, Any]] | None worker_attestation: str | None locked_guide_version: str | None - locked_review_policy_version: str | None - locked_revision_policy_version: str | None + locked_review_policy_id: str | None + locked_review_policy_generation: int | None + locked_review_policy_hash: str | None + locked_revision_policy_id: str | None + locked_revision_policy_generation: int | None + locked_revision_policy_hash: str | None locked_payment_policy_version: str | None locked_guide_source_snapshot_id: str | None locked_guide_source_snapshot_hash: str | None diff --git a/backend/app/modules/tasks/service.py b/backend/app/modules/tasks/service.py index 8945ac0d5..459ce1b49 100644 --- a/backend/app/modules/tasks/service.py +++ b/backend/app/modules/tasks/service.py @@ -108,8 +108,12 @@ CONTRIBUTOR_VISIBLE_AUDIT_PAYLOAD_KEYS = { "assignment_id", "locked_guide_version", - "locked_review_policy_version", - "locked_revision_policy_version", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", "locked_payment_policy_version", "source_type", "submission_id", @@ -137,8 +141,12 @@ "locked_post_submit_checker_policy_version", "locked_post_submit_checker_policy_hash", "locked_post_submit_checker_policy_body", - "locked_review_policy_version", - "locked_revision_policy_version", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", "locked_payment_policy_version", "locked_guide_source_snapshot_id", "locked_guide_source_snapshot_hash", @@ -395,18 +403,14 @@ async def get_task_work_context( task = await self._get_task(task_id) await self._ensure_task_visible(actor, task) context = await self._load_locked_task_context(task) - eligibility = ( - await self._legacy_workflow_eligibility.get_active_submitter_eligibility( - actor.actor_id - ) + eligibility = await self._legacy_workflow_eligibility.get_active_submitter_eligibility( + actor.actor_id ) return self._work_context_response( actor, task, context, - has_active_submitter_eligibility=( - "worker" in actor.roles and eligibility is not None - ), + has_active_submitter_eligibility=("worker" in actor.roles and eligibility is not None), ) async def get_task_submission_requirements( @@ -744,8 +748,12 @@ async def create_submission( ), locked_post_submit_checker_policy_hash=task.locked_post_submit_checker_policy_hash, locked_post_submit_checker_policy_body=task.locked_post_submit_checker_policy_body, - locked_review_policy_version=task.locked_review_policy_version, - locked_revision_policy_version=task.locked_revision_policy_version, + locked_review_policy_id=task.locked_review_policy_id, + locked_review_policy_generation=task.locked_review_policy_generation, + locked_review_policy_hash=task.locked_review_policy_hash, + locked_revision_policy_id=task.locked_revision_policy_id, + locked_revision_policy_generation=task.locked_revision_policy_generation, + locked_revision_policy_hash=task.locked_revision_policy_hash, locked_payment_policy_version=task.locked_payment_policy_version, locked_guide_source_snapshot_id=task.locked_guide_source_snapshot_id, locked_guide_source_snapshot_hash=task.locked_guide_source_snapshot_hash, @@ -1199,9 +1207,7 @@ async def _require_active_contributor(self, actor: ActorContext) -> None: raise ActiveContributorRequired(ActiveContributorRequired.message) from exc except (CanonicalWriteActorUnavailable, SQLAlchemyError) as exc: await self._session.rollback() - raise ContributorIdentityUnavailable( - ContributorIdentityUnavailable.message - ) from exc + raise ContributorIdentityUnavailable(ContributorIdentityUnavailable.message) from exc @staticmethod def _submission_audit_payload(submission: Submission) -> dict: @@ -1400,10 +1406,8 @@ async def _load_locked_task_context(self, task: WorkstreamTask) -> LockedTaskCon {"field": "locked_guide_source_snapshot_hash"}, ) - effective_policy = ( - await self._project_repo.get_effective_submission_artifact_policy_by_id( - task.locked_effective_project_submission_artifact_policy_id or "", - ) + effective_policy = await self._project_repo.get_effective_submission_artifact_policy_by_id( + task.locked_effective_project_submission_artifact_policy_id or "", ) if ( effective_policy is None @@ -1467,10 +1471,8 @@ async def _load_locked_task_context(self, task: WorkstreamTask) -> LockedTaskCon {"field": "locked_pre_submit_checker_policy_id"}, ) - post_submit_checker_policy = ( - await self._project_repo.get_post_submit_checker_policy_by_id( - task.locked_post_submit_checker_policy_id or "", - ) + post_submit_checker_policy = await self._project_repo.get_post_submit_checker_policy_by_id( + task.locked_post_submit_checker_policy_id or "", ) if ( post_submit_checker_policy is None @@ -1478,8 +1480,7 @@ async def _load_locked_task_context(self, task: WorkstreamTask) -> LockedTaskCon or post_submit_checker_policy.guide_version != task.locked_post_submit_checker_policy_version or post_submit_checker_policy.guide_version != task.locked_guide_version - or post_submit_checker_policy.policy_hash - != task.locked_post_submit_checker_policy_hash + or post_submit_checker_policy.policy_hash != task.locked_post_submit_checker_policy_hash ): raise TaskLockedContextInvalid( "task locked post-submit checker policy is invalid", @@ -1508,13 +1509,11 @@ async def _load_locked_task_context(self, task: WorkstreamTask) -> LockedTaskCon blocking_severities=parsed_post_submit_body.blocking_severities, ) - review_policy = await self._project_repo.get_review_policy( - task.project_id, - task.locked_review_policy_version or "", + review_policy = await self._project_repo.get_review_policy_by_id( + task.locked_review_policy_id or "" ) - revision_policy = await self._project_repo.get_revision_policy( - task.project_id, - task.locked_revision_policy_version or "", + revision_policy = await self._project_repo.get_revision_policy_by_id( + task.locked_revision_policy_id or "" ) payment_policy = await self._project_repo.get_payment_policy( task.project_id, @@ -1522,9 +1521,15 @@ async def _load_locked_task_context(self, task: WorkstreamTask) -> LockedTaskCon ) if ( review_policy is None + or review_policy.project_id != task.project_id or review_policy.guide_version != task.locked_guide_version + or review_policy.policy_generation != task.locked_review_policy_generation + or review_policy.policy_hash != task.locked_review_policy_hash or revision_policy is None + or revision_policy.project_id != task.project_id or revision_policy.guide_version != task.locked_guide_version + or revision_policy.policy_generation != task.locked_revision_policy_generation + or revision_policy.policy_hash != task.locked_revision_policy_hash or payment_policy is None or payment_policy.guide_version != task.locked_guide_version ): @@ -1548,11 +1553,7 @@ async def _load_locked_task_context(self, task: WorkstreamTask) -> LockedTaskCon def _missing_locked_context_fields(self, task: WorkstreamTask) -> list[str]: """Return missing locked-context fields for a task.""" - return [ - field - for field in LOCKED_CONTEXT_REQUIRED_FIELDS - if not getattr(task, field) - ] + return [field for field in LOCKED_CONTEXT_REQUIRED_FIELDS if not getattr(task, field)] def _work_context_response( self, @@ -1579,10 +1580,14 @@ def _work_context_response( effective_at=context.guide.effective_at, ), review_policy=TaskReviewPolicyContext( - guide_version=task.locked_review_policy_version or "", + policy_id=task.locked_review_policy_id or "", + policy_generation=task.locked_review_policy_generation or 0, + policy_hash=task.locked_review_policy_hash or "", ), revision_policy=TaskRevisionPolicyContext( - guide_version=task.locked_revision_policy_version or "", + policy_id=task.locked_revision_policy_id or "", + policy_generation=task.locked_revision_policy_generation or 0, + policy_hash=task.locked_revision_policy_hash or "", ), payment_policy=TaskPaymentPolicyContext( guide_version=task.locked_payment_policy_version or "", @@ -1885,20 +1890,20 @@ def _locked_context_response( locked_pre_submit_checker_bundle_hash=( task.locked_pre_submit_checker_bundle_hash or "" ), - locked_post_submit_checker_policy_id=( - task.locked_post_submit_checker_policy_id or "" - ), + locked_post_submit_checker_policy_id=(task.locked_post_submit_checker_policy_id or ""), locked_post_submit_checker_policy_version=( task.locked_post_submit_checker_policy_version or "" ), locked_post_submit_checker_policy_hash=( task.locked_post_submit_checker_policy_hash or "" ), - locked_post_submit_checker_policy_body_summary=( - context.locked_post_submit_policy_body - ), - locked_review_policy_version=task.locked_review_policy_version or "", - locked_revision_policy_version=task.locked_revision_policy_version or "", + locked_post_submit_checker_policy_body_summary=(context.locked_post_submit_policy_body), + locked_review_policy_id=task.locked_review_policy_id or "", + locked_review_policy_generation=task.locked_review_policy_generation or 0, + locked_review_policy_hash=task.locked_review_policy_hash or "", + locked_revision_policy_id=task.locked_revision_policy_id or "", + locked_revision_policy_generation=task.locked_revision_policy_generation or 0, + locked_revision_policy_hash=task.locked_revision_policy_hash or "", locked_payment_policy_version=task.locked_payment_policy_version or "", ) @@ -2037,8 +2042,12 @@ def _stamp_locked_context( task.locked_post_submit_checker_policy_version = checker_policy.guide_version task.locked_post_submit_checker_policy_hash = checker_policy.policy_hash task.locked_post_submit_checker_policy_body = dict(checker_policy.policy_body or {}) - task.locked_review_policy_version = review_policy.guide_version - task.locked_revision_policy_version = revision_policy.guide_version + task.locked_review_policy_id = review_policy.id + task.locked_review_policy_generation = review_policy.policy_generation + task.locked_review_policy_hash = review_policy.policy_hash + task.locked_revision_policy_id = revision_policy.id + task.locked_revision_policy_generation = revision_policy.policy_generation + task.locked_revision_policy_hash = revision_policy.policy_hash task.locked_payment_policy_version = payment_policy.guide_version task.locked_guide_source_snapshot_id = source_snapshot.id task.locked_guide_source_snapshot_hash = source_snapshot.bundle_hash @@ -2047,9 +2056,7 @@ def _stamp_locked_context( effective_policy.effective_policy_hash ) task.locked_pre_submit_checker_policy_id = pre_submit_checker_policy.id - task.locked_pre_submit_checker_bundle_hash = ( - pre_submit_checker_policy.compiled_bundle_hash - ) + task.locked_pre_submit_checker_bundle_hash = pre_submit_checker_policy.compiled_bundle_hash task.base_amount = payment_policy.base_amount task.currency = payment_policy.currency task.payout_type = payment_policy.payout_type @@ -2184,8 +2191,12 @@ async def _write_task_audit( task.locked_post_submit_checker_policy_version ), "locked_post_submit_checker_policy_hash": task.locked_post_submit_checker_policy_hash, - "locked_review_policy_version": task.locked_review_policy_version, - "locked_revision_policy_version": task.locked_revision_policy_version, + "locked_review_policy_id": task.locked_review_policy_id, + "locked_review_policy_generation": task.locked_review_policy_generation, + "locked_review_policy_hash": task.locked_review_policy_hash, + "locked_revision_policy_id": task.locked_revision_policy_id, + "locked_revision_policy_generation": task.locked_revision_policy_generation, + "locked_revision_policy_hash": task.locked_revision_policy_hash, "locked_payment_policy_version": task.locked_payment_policy_version, "locked_guide_source_snapshot_id": task.locked_guide_source_snapshot_id, "locked_guide_source_snapshot_hash": task.locked_guide_source_snapshot_hash, @@ -2290,8 +2301,12 @@ def _submission_response( response.artifact_hash_manifest = None response.worker_attestation = None response.locked_guide_version = None - response.locked_review_policy_version = None - response.locked_revision_policy_version = None + response.locked_review_policy_id = None + response.locked_review_policy_generation = None + response.locked_review_policy_hash = None + response.locked_revision_policy_id = None + response.locked_revision_policy_generation = None + response.locked_revision_policy_hash = None response.locked_payment_policy_version = None response.locked_guide_source_snapshot_id = None response.locked_guide_source_snapshot_hash = None diff --git a/backend/scripts/api_contract_e2e.py b/backend/scripts/api_contract_e2e.py index 1c9312c01..ae72163b3 100644 --- a/backend/scripts/api_contract_e2e.py +++ b/backend/scripts/api_contract_e2e.py @@ -34,10 +34,16 @@ PaymentPolicy, PostSubmitCheckerPolicy, PreSubmitCheckerPolicy, + ProjectGuide, ProjectSetupRun, ReviewPolicy, RevisionPolicy, ) +from app.modules.projects.policy_lineage import ( + ReviewPolicySemantics, + RevisionPolicySemantics, + policy_digest, +) from app.modules.projects.post_submit_policy import ( build_project_post_submit_checker_spec, compile_project_post_submit_checker_spec, @@ -650,26 +656,41 @@ async def seed_pending_policy_boundaries(project_id: str, guide_version: str) -> each dedicated policy boundary is activated. """ async with db_session.get_session_factory()() as session: + review_id = str(uuid4()) + revision_id = str(uuid4()) + review_semantics = ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + allowed_decisions=("accept", "needs_revision", "reject"), + minimum_finding_fields=("issue", "required_fix"), + ) + revision_semantics = RevisionPolicySemantics( + max_revision_rounds=7, + revision_deadline_hours=48, + allowed_resubmission_states=("needs_revision",), + reviewer_reassignment_rule="same reviewer preferred", + ) + review_hash = policy_digest("review", review_semantics) + revision_hash = policy_digest("revision", revision_semantics) session.add_all( [ ReviewPolicy( - id=str(uuid4()), + id=review_id, project_id=project_id, guide_version=guide_version, - requires_second_review=False, - allowed_decisions=["accept", "needs_revision", "reject"], - minimum_finding_fields=["issue", "required_fix"], - sla_hours=24, + policy_generation=1, + policy_hash=review_hash, + semantics_status="complete", + **review_semantics.model_dump(mode="python"), ), RevisionPolicy( - id=str(uuid4()), + id=revision_id, project_id=project_id, guide_version=guide_version, - max_revision_rounds=7, - revision_deadline_hours=48, - auto_reject_after_limit=True, - allowed_resubmission_states=["needs_revision"], - reviewer_reassignment_rule="same reviewer preferred", + policy_generation=1, + policy_hash=revision_hash, + semantics_status="complete", + **revision_semantics.model_dump(mode="python"), ), PaymentPolicy( id=str(uuid4()), @@ -684,6 +705,35 @@ async def seed_pending_policy_boundaries(project_id: str, guide_version: str) -> ), ] ) + await session.flush() + guide = await session.scalar( + select(ProjectGuide).where( + ProjectGuide.project_id == project_id, + ProjectGuide.version == guide_version, + ) + ) + if guide is None: + raise RuntimeError("policy seed requires its exact project guide") + await session.execute( + text("alter table project_guides disable trigger guide_mutation_product_custody") + ) + await session.execute( + text("alter table project_guides disable trigger guide_lineage_lifecycle_guard") + ) + guide.selected_review_policy_id = review_id + guide.selected_review_policy_generation = 1 + guide.selected_review_policy_hash = review_hash + guide.selected_revision_policy_id = revision_id + guide.selected_revision_policy_generation = 1 + guide.selected_revision_policy_hash = revision_hash + await session.flush() + await session.execute(text("set constraints all immediate")) + await session.execute( + text("alter table project_guides enable trigger guide_lineage_lifecycle_guard") + ) + await session.execute( + text("alter table project_guides enable trigger guide_mutation_product_custody") + ) await session.commit() @@ -1573,12 +1623,12 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: manager_token, {"reason": "real API screening passed"}, ) - assert { - screened["locked_guide_version"], - screened["locked_review_policy_version"], - screened["locked_revision_policy_version"], - screened["locked_payment_policy_version"], - } == {"v1"} + assert screened["locked_guide_version"] == "v1" + assert screened["locked_review_policy_generation"] == 1 + assert screened["locked_review_policy_hash"].startswith("sha256:") + assert screened["locked_revision_policy_generation"] == 1 + assert screened["locked_revision_policy_hash"].startswith("sha256:") + assert screened["locked_payment_policy_version"] == "v1" assert screened["base_amount"] == "25.00" assert screened["currency"] == "USD" assert screened["payout_type"] == "fixed" @@ -1988,8 +2038,12 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: "package_hash", "worker_attestation", "locked_guide_version", - "locked_review_policy_version", - "locked_revision_policy_version", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", "locked_payment_policy_version", "locked_post_submit_checker_policy_hash", ): @@ -2026,12 +2080,14 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: manager_token, ) assert locked["finalized_at"] is not None - assert { - locked["locked_guide_version"], - locked["locked_review_policy_version"], - locked["locked_revision_policy_version"], - locked["locked_payment_policy_version"], - } == {"v1"} + assert locked["locked_guide_version"] == "v1" + assert locked["locked_review_policy_id"] == screened["locked_review_policy_id"] + assert locked["locked_review_policy_generation"] == 1 + assert locked["locked_review_policy_hash"] == screened["locked_review_policy_hash"] + assert locked["locked_revision_policy_id"] == screened["locked_revision_policy_id"] + assert locked["locked_revision_policy_generation"] == 1 + assert locked["locked_revision_policy_hash"] == screened["locked_revision_policy_hash"] + assert locked["locked_payment_policy_version"] == "v1" assert all(item["finalized_at"] == locked["finalized_at"] for item in locked["evidence_items"]) checker_run = await wait_for_submission_checker_run(client, manager_token, submission["id"]) assert checker_run["routing_recommendation"] == "allow_review" diff --git a/backend/scripts/week2_api_e2e.py b/backend/scripts/week2_api_e2e.py index 8d61d581d..10b79ab7a 100644 --- a/backend/scripts/week2_api_e2e.py +++ b/backend/scripts/week2_api_e2e.py @@ -505,13 +505,14 @@ def assert_finalized_submission_versions(submission: dict) -> None: submission: Locked submission response payload. """ ensure(submission["finalized_at"] is not None, "locked submission missing finalized_at") - locked_versions = { - submission["locked_guide_version"], - submission["locked_review_policy_version"], - submission["locked_revision_policy_version"], - submission["locked_payment_policy_version"], - } - ensure(locked_versions == {"v1"}, f"locked context drifted: {locked_versions}") + ensure(submission["locked_guide_version"] == "v1", "locked guide context drifted") + ensure(submission["locked_review_policy_id"], "locked review policy id missing") + ensure(submission["locked_review_policy_generation"] == 1, "review generation drifted") + ensure(submission["locked_review_policy_hash"], "locked review policy hash missing") + ensure(submission["locked_revision_policy_id"], "locked revision policy id missing") + ensure(submission["locked_revision_policy_generation"] == 1, "revision generation drifted") + ensure(submission["locked_revision_policy_hash"], "locked revision policy hash missing") + ensure(submission["locked_payment_policy_version"] == "v1", "payment context drifted") ensure( all(item["finalized_at"] == submission["finalized_at"] for item in submission["evidence_items"]), "evidence item finalized_at does not match submission finalized_at", @@ -806,13 +807,14 @@ async def assert_week2_database_invariants(scenarios: list[dict]) -> None: task.status == scenario["expected_final_task_status"], f"{scenario['name']} final task status drifted: {task.status}", ) - task_versions = { - task.locked_guide_version, - task.locked_review_policy_version, - task.locked_revision_policy_version, - task.locked_payment_policy_version, - } - ensure(task_versions == {"v1"}, f"{scenario['name']} task context drifted") + ensure(task.locked_guide_version == "v1", f"{scenario['name']} guide drifted") + ensure(task.locked_review_policy_id, f"{scenario['name']} review id missing") + ensure(task.locked_review_policy_generation == 1, "review generation drifted") + ensure(task.locked_review_policy_hash, f"{scenario['name']} review hash missing") + ensure(task.locked_revision_policy_id, f"{scenario['name']} revision id missing") + ensure(task.locked_revision_policy_generation == 1, "revision generation drifted") + ensure(task.locked_revision_policy_hash, f"{scenario['name']} revision hash missing") + ensure(task.locked_payment_policy_version == "v1", "payment context drifted") ensure( task.locked_post_submit_checker_policy_id is not None and task.locked_post_submit_checker_policy_version == "v1" @@ -834,14 +836,23 @@ async def assert_week2_database_invariants(scenarios: list[dict]) -> None: and task.locked_pre_submit_checker_bundle_hash is not None, f"{scenario['name']} task pre-submit checker lock missing", ) - submission_versions = { - submission.locked_guide_version, - submission.locked_review_policy_version, - submission.locked_revision_policy_version, - submission.locked_payment_policy_version, - } ensure( - submission_versions == task_versions, + ( + submission.locked_review_policy_id, + submission.locked_review_policy_generation, + submission.locked_review_policy_hash, + submission.locked_revision_policy_id, + submission.locked_revision_policy_generation, + submission.locked_revision_policy_hash, + ) + == ( + task.locked_review_policy_id, + task.locked_review_policy_generation, + task.locked_review_policy_hash, + task.locked_revision_policy_id, + task.locked_revision_policy_generation, + task.locked_revision_policy_hash, + ), f"{scenario['name']} submission context drifted", ) ensure( @@ -922,13 +933,25 @@ async def assert_week2_database_invariants(scenarios: list[dict]) -> None: checker_run.package_hash == submission.package_hash, f"{scenario['name']} package hash drifted on checker run", ) - run_versions = { - checker_run.locked_guide_version, - checker_run.locked_review_policy_version, - checker_run.locked_revision_policy_version, - checker_run.locked_payment_policy_version, - } - ensure(run_versions == submission_versions, f"{scenario['name']} checker context drifted") + ensure( + ( + checker_run.locked_review_policy_id, + checker_run.locked_review_policy_generation, + checker_run.locked_review_policy_hash, + checker_run.locked_revision_policy_id, + checker_run.locked_revision_policy_generation, + checker_run.locked_revision_policy_hash, + ) + == ( + submission.locked_review_policy_id, + submission.locked_review_policy_generation, + submission.locked_review_policy_hash, + submission.locked_revision_policy_id, + submission.locked_revision_policy_generation, + submission.locked_revision_policy_hash, + ), + f"{scenario['name']} checker context drifted", + ) ensure( checker_run.locked_post_submit_checker_policy_id == submission.locked_post_submit_checker_policy_id, diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index d8b3f9a97..a486d4d21 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -21,7 +21,7 @@ from scripts.run_isolated_tests import LOOPBACK, NAME_RE, ROLE_RE DDL_LOCK_DIRECTORY = Path("/tmp") -EXPECTED_PUBLIC_SCHEMA_SHA256 = "e5e55ca12f13d860d2c9f374376b8bd4c0c981103ee242246fa0389b0a5b5cb7" +EXPECTED_PUBLIC_SCHEMA_SHA256 = "36d0f42dbf966bd0ccab2a1e267e15badc1b7f077756c14feb74d8522bd49365" PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", "alembic_version", @@ -95,6 +95,8 @@ "project_create_idempotency_records", "project_role_grants", "project_role_qualification_snapshots", + "review_policies", + "revision_policies", ) TestDatabaseReset = Callable[..., Awaitable[None]] DatabaseLock = Callable[[], AbstractContextManager[None]] diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 8468e1cc7..79835cc08 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -73,7 +73,7 @@ snapshot_existing_service_rows, ) -HEAD_REVISION = "0045_guide_metadata_authority" +HEAD_REVISION = "0046_policy_identity_lineage" pytestmark = pytest.mark.postgres_schema_contract @@ -11801,3 +11801,282 @@ async def _remove_project_role_table_blockers(database_url: str, ids: dict[str, await connection.execute(text(f"alter table {table} enable trigger user")) finally: await engine.dispose() + + +def test_xint003_02a_policy_lineage_backfill_immutability_and_roundtrip( + isolated_database_env: str, + migration_lock, +) -> None: + """Prove historical policies get exact identity without invented semantics.""" + project_root = Path(__file__).resolve().parents[1] + config = Config(str(project_root / "alembic.ini")) + config.set_main_option("script_location", str(project_root / "alembic")) + ids = { + "project": str(uuid4()), + "guide": str(uuid4()), + "review": str(uuid4()), + "revision": str(uuid4()), + } + + with migration_lock(): + try: + command.downgrade(config, "0045_guide_metadata_authority") + asyncio.run(_seed_xint003_02a_legacy_policies(isolated_database_env, ids)) + command.upgrade(config, "0046_policy_identity_lineage") + state = asyncio.run(_xint003_02a_policy_state(isolated_database_env, ids)) + immutable = asyncio.run( + _xint003_02a_policy_immutable_writes(isolated_database_env, ids) + ) + with pytest.raises( + RuntimeError, match="cannot downgrade populated immutable policy lineage" + ): + command.downgrade(config, "0045_guide_metadata_authority") + refused_state = asyncio.run( + _xint003_02a_policy_state(isolated_database_env, ids) + ) + finally: + asyncio.run(_remove_xint003_02a_immutable_policies(isolated_database_env, ids)) + command.downgrade(config, "0045_guide_metadata_authority") + command.upgrade(config, "head") + + assert state["review"][0:3] == (ids["review"], 1, "legacy_incomplete") + assert state["revision"][0:3] == (ids["revision"], 1, "legacy_incomplete") + assert state["review"][3].startswith("sha256:") + assert state["revision"][3].startswith("sha256:") + assert state["guide"] == ( + ids["review"], + 1, + state["review"][3], + ids["revision"], + 1, + state["revision"][3], + ) + assert immutable == { + "partial_selection", + "active_selection_change", + "review_update", + "review_delete", + "review_truncate", + "revision_update", + "revision_delete", + "revision_truncate", + } + assert refused_state == state + + +async def _seed_xint003_02a_legacy_policies( + database_url: str, ids: dict[str, str] +) -> None: + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + for table in ("projects", "project_guides"): + await connection.execute(text(f"alter table {table} disable trigger user")) + await connection.execute( + text( + "insert into projects (id,name,slug,status) values " + "(:project,'XINT 003 02A','xint-003-02a','draft')" + ), + ids, + ) + await connection.execute( + text( + "insert into project_guides " + "(id,project_id,version,status,content_markdown,created_by) values " + "(:guide,:project,'v1','draft','# Legacy guide','migration-test')" + ), + ids, + ) + for table in reversed(("projects", "project_guides")): + await connection.execute(text(f"alter table {table} enable trigger user")) + await connection.execute( + text( + "insert into review_policies " + "(id,project_id,guide_version,requires_second_review,allowed_decisions," + "minimum_finding_fields,sla_hours) values " + "(:review,:project,'v1',false,'[\"accept\",\"needs_revision\"," + "\"reject\"]'::json,'[]'::json,24)" + ), + ids, + ) + await connection.execute( + text( + "insert into revision_policies " + "(id,project_id,guide_version,max_revision_rounds,revision_deadline_hours," + "auto_reject_after_limit,allowed_resubmission_states) values " + "(:revision,:project,'v1',3,48,false,'[\"needs_revision\"]'::json)" + ), + ids, + ) + finally: + await engine.dispose() + + +async def _xint003_02a_policy_state( + database_url: str, ids: dict[str, str] +) -> dict[str, tuple]: + engine = create_async_engine(database_url) + try: + async with engine.connect() as connection: + review = tuple( + ( + await connection.execute( + text( + "select id,policy_generation,semantics_status,policy_hash " + "from review_policies where id=:review" + ), + ids, + ) + ).one() + ) + revision = tuple( + ( + await connection.execute( + text( + "select id,policy_generation,semantics_status,policy_hash " + "from revision_policies where id=:revision" + ), + ids, + ) + ).one() + ) + guide = tuple( + ( + await connection.execute( + text( + "select selected_review_policy_id,selected_review_policy_generation," + "selected_review_policy_hash,selected_revision_policy_id," + "selected_revision_policy_generation,selected_revision_policy_hash " + "from project_guides where id=:guide" + ), + ids, + ) + ).one() + ) + return {"review": review, "revision": revision, "guide": guide} + finally: + await engine.dispose() + + +async def _xint003_02a_policy_immutable_writes( + database_url: str, ids: dict[str, str] +) -> set[str]: + engine = create_async_engine(database_url) + refused: set[str] = set() + try: + async with engine.connect() as connection: + transaction = await connection.begin() + with pytest.raises(IntegrityError): + await connection.execute( + text( + "update project_guides set selected_review_policy_hash=null " + "where id=:guide" + ), + ids, + ) + refused.add("partial_selection") + await transaction.rollback() + async with engine.begin() as connection: + await connection.execute( + text("alter table project_guides disable trigger guide_mutation_product_custody") + ) + await connection.execute( + text("alter table project_guides disable trigger guide_lineage_lifecycle_guard") + ) + await connection.execute( + text("update project_guides set status='active' where id=:guide"), ids + ) + await connection.execute( + text("alter table project_guides enable trigger guide_mutation_product_custody") + ) + await connection.execute( + text("alter table project_guides enable trigger guide_lineage_lifecycle_guard") + ) + async with engine.connect() as connection: + transaction = await connection.begin() + with pytest.raises(DBAPIError): + await connection.execute( + text( + "update project_guides set selected_review_policy_hash=:hash " + "where id=:guide" + ), + ids | {"hash": "sha256:" + "f" * 64}, + ) + refused.add("active_selection_change") + await transaction.rollback() + statements = { + "review_update": ( + "update review_policies set requires_second_review=true where id=:review", + ids, + ), + "review_delete": ("delete from review_policies where id=:review", ids), + "review_truncate": ("truncate review_policies", {}), + "revision_update": ( + "update revision_policies set max_revision_rounds=4 where id=:revision", + ids, + ), + "revision_delete": ("delete from revision_policies where id=:revision", ids), + "revision_truncate": ("truncate revision_policies", {}), + } + for operation, (sql, params) in statements.items(): + async with engine.connect() as connection: + transaction = await connection.begin() + with pytest.raises(DBAPIError): + await connection.execute(text(sql), params) + refused.add(operation) + await transaction.rollback() + return refused + finally: + await engine.dispose() + + +async def _remove_xint003_02a_immutable_policies( + database_url: str, ids: dict[str, str] +) -> None: + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + has_lineage = bool( + await connection.scalar( + text( + "select exists(select 1 from information_schema.columns " + "where table_schema='public' and table_name='project_guides' " + "and column_name='selected_review_policy_id')" + ) + ) + ) + for table in ( + "projects", + "project_guides", + "review_policies", + "revision_policies", + ): + await connection.execute(text(f"alter table {table} disable trigger user")) + if has_lineage: + await connection.execute( + text( + "update project_guides set status='draft',selected_review_policy_id=null," + "selected_review_policy_generation=null,selected_review_policy_hash=null," + "selected_revision_policy_id=null," + "selected_revision_policy_generation=null," + "selected_revision_policy_hash=null where id=:guide" + ), + ids, + ) + await connection.execute(text("delete from review_policies where id=:review"), ids) + await connection.execute( + text("delete from revision_policies where id=:revision"), ids + ) + await connection.execute(text("delete from project_guides where id=:guide"), ids) + await connection.execute(text("delete from projects where id=:project"), ids) + for table in reversed( + ( + "projects", + "project_guides", + "review_policies", + "revision_policies", + ) + ): + await connection.execute(text(f"alter table {table} enable trigger user")) + finally: + await engine.dispose() diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index 2bb996530..e01a380e3 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -102,6 +102,11 @@ RevisionPolicy, SubmissionArtifactPolicy, ) +from app.modules.projects.policy_lineage import ( + ReviewPolicySemantics, + RevisionPolicySemantics, + policy_digest, +) from project_create_fixtures import seed_historical_project, suspend_historical_product_custody from app.modules.tasks.models import AuditEvent, Submission, WorkstreamTask from tests.artifact_store_helpers import ( @@ -427,6 +432,8 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: effective_policy_id = str(uuid4()) pre_submit_policy_id = str(uuid4()) post_submit_policy_id = str(uuid4()) + review_policy_id = str(uuid4()) + revision_policy_id = str(uuid4()) task_id = str(uuid4()) submission_id = str(uuid4()) contributor_id = str(uuid4()) @@ -443,6 +450,22 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: post_submit_policy_body = {"required_checkers": []} post_submit_policy_hash = canonical_json_hash(post_submit_policy_body) now = datetime.now(UTC) + review_hash = policy_digest( + "review", + ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + allowed_decisions=("accept", "needs_revision", "reject"), + ), + ) + revision_hash = policy_digest( + "revision", + RevisionPolicySemantics( + max_revision_rounds=1, + revision_deadline_hours=24, + allowed_resubmission_states=("needs_revision",), + ), + ) await seed_historical_project( session, @@ -571,20 +594,31 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: created_by="setup-actor", ), ReviewPolicy( - id=str(uuid4()), + id=review_policy_id, project_id=project_id, guide_version=guide_version, + policy_generation=1, + policy_hash=review_hash, + semantics_status="complete", + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + max_active_review_leases_per_reviewer=1, + self_review_allowed=False, + reject_policy="close_task", + finding_evidence_requirement="optional", requires_second_review=False, allowed_decisions=["accept", "needs_revision", "reject"], minimum_finding_fields=[], ), RevisionPolicy( - id=str(uuid4()), + id=revision_policy_id, project_id=project_id, guide_version=guide_version, + policy_generation=1, + policy_hash=revision_hash, + semantics_status="complete", max_revision_rounds=1, revision_deadline_hours=24, - auto_reject_after_limit=True, allowed_resubmission_states=["needs_revision"], ), PaymentPolicy( @@ -595,6 +629,20 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: ] ) await session.flush() + async with suspend_historical_product_custody( + session, + table="project_guides", + triggers=("guide_mutation_product_custody",), + ): + guide = await session.get(ProjectGuide, guide_id) + assert guide is not None + guide.selected_review_policy_id = review_policy_id + guide.selected_review_policy_generation = 1 + guide.selected_review_policy_hash = review_hash + guide.selected_revision_policy_id = revision_policy_id + guide.selected_revision_policy_generation = 1 + guide.selected_revision_policy_hash = revision_hash + await session.flush() session.add( WorkstreamTask( id=task_id, @@ -604,8 +652,12 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: locked_post_submit_checker_policy_version=guide_version, locked_post_submit_checker_policy_hash=post_submit_policy_hash, locked_post_submit_checker_policy_body=post_submit_policy_body, - locked_review_policy_version=guide_version, - locked_revision_policy_version=guide_version, + locked_review_policy_id=review_policy_id, + locked_review_policy_generation=1, + locked_review_policy_hash=review_hash, + locked_revision_policy_id=revision_policy_id, + locked_revision_policy_generation=1, + locked_revision_policy_hash=revision_hash, locked_payment_policy_version=guide_version, locked_guide_source_snapshot_id=snapshot_id, locked_guide_source_snapshot_hash=snapshot_hash, @@ -660,8 +712,12 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: locked_post_submit_checker_policy_version=guide_version, locked_post_submit_checker_policy_hash=post_submit_policy_hash, locked_post_submit_checker_policy_body=post_submit_policy_body, - locked_review_policy_version=guide_version, - locked_revision_policy_version=guide_version, + locked_review_policy_id=review_policy_id, + locked_review_policy_generation=1, + locked_review_policy_hash=review_hash, + locked_revision_policy_id=revision_policy_id, + locked_revision_policy_generation=1, + locked_revision_policy_hash=revision_hash, locked_payment_policy_version=guide_version, locked_guide_source_snapshot_id=snapshot_id, locked_guide_source_snapshot_hash=snapshot_hash, @@ -693,8 +749,12 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: locked_post_submit_checker_policy_version=guide_version, locked_post_submit_checker_policy_hash=post_submit_policy_hash, locked_post_submit_checker_policy_body=post_submit_policy_body, - locked_review_policy_version=guide_version, - locked_revision_policy_version=guide_version, + locked_review_policy_id=review_policy_id, + locked_review_policy_generation=1, + locked_review_policy_hash=review_hash, + locked_revision_policy_id=revision_policy_id, + locked_revision_policy_generation=1, + locked_revision_policy_hash=revision_hash, locked_payment_policy_version=guide_version, package_hash=canonical_json_hash({"submission": submission_id}), artifact_hash_manifest=[], @@ -4143,8 +4203,16 @@ async def test_checker_output_requires_exact_active_fixed_service_identity( locked_post_submit_checker_policy_body=( canonical_task.locked_post_submit_checker_policy_body ), - locked_review_policy_version=canonical_task.locked_review_policy_version, - locked_revision_policy_version=(canonical_task.locked_revision_policy_version), + locked_review_policy_id=canonical_task.locked_review_policy_id, + locked_review_policy_generation=( + canonical_task.locked_review_policy_generation + ), + locked_review_policy_hash=canonical_task.locked_review_policy_hash, + locked_revision_policy_id=canonical_task.locked_revision_policy_id, + locked_revision_policy_generation=( + canonical_task.locked_revision_policy_generation + ), + locked_revision_policy_hash=(canonical_task.locked_revision_policy_hash), locked_payment_policy_version=(canonical_task.locked_payment_policy_version), locked_guide_source_snapshot_id=( canonical_task.locked_guide_source_snapshot_id @@ -4404,9 +4472,7 @@ async def seed_attempt_only() -> None: async with factory() as session: context = _context() await _seed_human_actor(session, context) - project_id, guide_id, snapshot_id, item_id = ( - str(uuid4()) for _ in range(4) - ) + project_id, guide_id, snapshot_id, item_id = (str(uuid4()) for _ in range(4)) await session.execute( text( "insert into projects (id,name,slug,status) values " diff --git a/backend/tests/test_checkers.py b/backend/tests/test_checkers.py index c804f5ab9..eee73c2c8 100644 --- a/backend/tests/test_checkers.py +++ b/backend/tests/test_checkers.py @@ -1290,6 +1290,32 @@ async def test_locked_submission_checker_run_persists_results_and_allows_review( == checker_run.locked_post_submit_checker_policy_hash == expected_post_submit_policy["policy_hash"] ) + assert ( + task.locked_review_policy_id, + task.locked_review_policy_generation, + task.locked_review_policy_hash, + ) == ( + submission.locked_review_policy_id, + submission.locked_review_policy_generation, + submission.locked_review_policy_hash, + ) == ( + checker_run.locked_review_policy_id, + checker_run.locked_review_policy_generation, + checker_run.locked_review_policy_hash, + ) + assert ( + task.locked_revision_policy_id, + task.locked_revision_policy_generation, + task.locked_revision_policy_hash, + ) == ( + submission.locked_revision_policy_id, + submission.locked_revision_policy_generation, + submission.locked_revision_policy_hash, + ) == ( + checker_run.locked_revision_policy_id, + checker_run.locked_revision_policy_generation, + checker_run.locked_revision_policy_hash, + ) audit_response = await checker_client.get( f"/api/v1/tasks/{started_task['id']}/audit-events", headers=auth_headers(), @@ -1562,6 +1588,37 @@ async def test_database_rejects_mismatched_submission_post_submit_policy_context assert results != [] +async def test_database_rejects_checker_run_with_another_tasks_submission( + checker_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project = await create_active_project(checker_client) + first_task = await create_started_task(checker_client, project["id"], monkeypatch) + first = await checker_client.post( + f"/api/v1/tasks/{first_task['id']}/submissions", + headers=auth_headers(), + json=complete_submission_payload(), + ) + second_task = await create_started_task( + checker_client, project["id"], monkeypatch, subject="worker-two" + ) + second = await checker_client.post( + f"/api/v1/tasks/{second_task['id']}/submissions", + headers=auth_headers(), + json=complete_submission_payload(), + ) + assert first.status_code == second.status_code == 201 + + async with db_session.get_session_factory()() as session: + checker_run = await session.scalar( + select(CheckerRun).where(CheckerRun.submission_id == first.json()["id"]) + ) + assert checker_run is not None + checker_run.task_id = second_task["id"] + with pytest.raises(IntegrityError): + await session.commit() + + async def test_locked_submission_checker_run_enforces_required_evidence_key( checker_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, @@ -2577,8 +2634,15 @@ async def test_worker_can_read_only_worker_visible_checker_result_fields( assert "locked_post_submit_checker_policy_version" not in body assert "locked_post_submit_checker_policy_hash" not in body assert "locked_post_submit_checker_policy_body" not in body - assert "locked_review_policy_version" not in body - assert "locked_revision_policy_version" not in body + for field in ( + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", + ): + assert field not in body assert "locked_payment_policy_version" not in body assert "package_hash" not in body assert "artifact_hash_manifest" not in body diff --git a/backend/tests/test_policy_identity_lineage.py b/backend/tests/test_policy_identity_lineage.py new file mode 100644 index 000000000..b7b34eb39 --- /dev/null +++ b/backend/tests/test_policy_identity_lineage.py @@ -0,0 +1,126 @@ +"""Focused proof for immutable review/revision policy identity.""" + +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from app.modules.authorization.catalogue import ACTION_BY_ID, ActionAvailability, ActionId +from app.modules.projects.policy_lineage import ( + ReviewPolicySemantics, + RevisionPolicySemantics, + policy_digest, + require_complete_policy, +) + + +def test_xint003_02a_policy_digests_are_typed_and_domain_separated() -> None: + review = ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + allowed_decisions=("accept", "needs_revision", "reject"), + ) + revision = RevisionPolicySemantics( + max_revision_rounds=3, + revision_deadline_hours=48, + allowed_resubmission_states=("needs_revision",), + ) + + review_hash = policy_digest("review", review) + revision_hash = policy_digest("revision", revision) + + assert review_hash.startswith("sha256:") + assert revision_hash.startswith("sha256:") + assert review_hash != revision_hash + + +@pytest.mark.parametrize( + ("status", "policy_hash", "values"), + [ + ("legacy_incomplete", "sha256:" + "a" * 64, {"lease": None}), + ("complete", None, {"lease": 1800}), + ("complete", "sha256:" + "a" * 64, {"lease": None}), + ], +) +def test_xint003_02a_incomplete_historical_policy_fails_closed( + status: str, policy_hash: str | None, values: dict[str, int | None] +) -> None: + with pytest.raises(ValueError, match="policy semantics are incomplete"): + require_complete_policy( + kind="review", + status=status, # type: ignore[arg-type] + policy_hash=policy_hash, + semantic_values=values, + ) + + +def test_xint003_02a_complete_policy_is_ready() -> None: + semantics = ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + allowed_decisions=("accept", "needs_revision", "reject"), + ) + require_complete_policy( + kind="review", + status="complete", + policy_hash=policy_digest("review", semantics), + semantic_values=semantics.model_dump(mode="python"), + ) + + +def test_xint003_02a_complete_policy_rejects_digest_mismatch() -> None: + semantics = ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + allowed_decisions=("accept", "needs_revision", "reject"), + ) + with pytest.raises(ValueError, match="digest mismatch"): + require_complete_policy( + kind="review", + status="complete", + policy_hash="sha256:" + "a" * 64, + semantic_values=semantics.model_dump(mode="python"), + ) + + +def test_xint003_02a_optional_revision_reassignment_remains_complete() -> None: + semantics = RevisionPolicySemantics( + max_revision_rounds=3, + revision_deadline_hours=48, + allowed_resubmission_states=("needs_revision",), + reviewer_reassignment_rule=None, + ) + require_complete_policy( + kind="revision", + status="complete", + policy_hash=policy_digest("revision", semantics), + semantic_values=semantics.model_dump(mode="python"), + ) + + +def test_xint003_02a_fixed_v01_review_guards_cannot_be_weakened() -> None: + with pytest.raises(ValidationError): + ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + max_active_review_leases_per_reviewer=2, # type: ignore[arg-type] + allowed_decisions=("accept",), + ) + with pytest.raises(ValidationError): + ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + self_review_allowed=True, # type: ignore[arg-type] + allowed_decisions=("accept",), + ) + + +def test_xint003_02a_policy_actions_remain_unavailable() -> None: + assert ( + ACTION_BY_ID[ActionId.PROJECT_REVIEW_POLICY_UPDATE].availability + is ActionAvailability.PLANNED + ) + assert ( + ACTION_BY_ID[ActionId.PROJECT_REVISION_POLICY_UPDATE].availability + is ActionAvailability.PLANNED + ) diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index d32ddaa8c..4cc7b0450 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -62,6 +62,11 @@ ReviewPolicy, SubmissionArtifactPolicy, ) +from app.modules.projects.policy_lineage import ( + ReviewPolicySemantics, + RevisionPolicySemantics, + policy_digest, +) from app.modules.projects.guide_mutation_repository import GuideMutationRepository from app.modules.tasks.models import AuditEvent from app.modules.authorization.models import ( @@ -185,9 +190,7 @@ def __init__(self) -> None: async def execute(self, statement: Any) -> Any: self.statements.append(statement) - return types.SimpleNamespace( - scalars=lambda: types.SimpleNamespace(all=lambda: []) - ) + return types.SimpleNamespace(scalars=lambda: types.SimpleNamespace(all=lambda: [])) class _PolicyReadRepository: @@ -486,9 +489,7 @@ def validate_activation_ready(self, *args: Any, **kwargs: Any) -> None: ) assert authorization.calls[-1][1].target_exists is False - repository.post_submit.pre_submit_checker_bundle_hash = ( - repository.checker.compiled_bundle_hash - ) + repository.post_submit.pre_submit_checker_bundle_hash = repository.checker.compiled_bundle_hash def raise_policy_setup_blocked(*_args: Any, **_kwargs: Any) -> None: raise PolicySetupBlocked("invalid canonical policy") @@ -674,9 +675,7 @@ async def create(_service, _resolved, _prepared, _key, _payload): session, # type: ignore[arg-type] ) assert returned is response - assert (session.commit_count, session.rollback_count) == ( - (0, 1) if replayed else (1, 0) - ) + assert (session.commit_count, session.rollback_count) == ((0, 1) if replayed else (1, 0)) @pytest.mark.asyncio @@ -963,9 +962,7 @@ async def test_project_diagnostic_read_composer_binds_each_action( source_snapshot_hash=f"sha256:{'a' * 64}", output_post_submit_checker_policy_id=None, ) - repository = _DiagnosticRepository( - project_id=project_id, guide_id=guide_id, target=target - ) + repository = _DiagnosticRepository(project_id=project_id, guide_id=guide_id, target=target) authorization = _DiagnosticAuthorization() result = await authorize_project_diagnostic_read( @@ -1024,9 +1021,7 @@ async def test_project_diagnostic_read_composer_fails_closed_for_invalid_or_miss guide_id=guide_id, ) repository.project = types.SimpleNamespace(id=project_id) - repository.guide = types.SimpleNamespace( - id=guide_id, project_id=str(uuid4()), version="v1" - ) + repository.guide = types.SimpleNamespace(id=guide_id, project_id=str(uuid4()), version="v1") with pytest.raises(RuntimeError, match="unexpectedly allowed"): await authorize_project_diagnostic_read( authorization=cast(Any, authorization), @@ -1047,9 +1042,7 @@ async def test_project_diagnostic_read_composer_locks_post_submit_policy_binding "source_snapshot_id": snapshot_id, "source_snapshot_hash": f"sha256:{'c' * 64}", } - run = types.SimpleNamespace( - id=run_id, output_post_submit_checker_policy_id=policy_id, **shared - ) + run = types.SimpleNamespace(id=run_id, output_post_submit_checker_policy_id=policy_id, **shared) policy = types.SimpleNamespace(id=policy_id, **shared) repository = _DiagnosticRepository(project_id=project_id, guide_id=guide_id, target=run) repository.post_policy = policy @@ -1216,9 +1209,7 @@ async def add_project_manager_admin_grant(project_id: str) -> UUID: return grant.id -async def add_local_admin_role_for_default_actor( - role: str, *, project_id: str | None -) -> UUID: +async def add_local_admin_role_for_default_actor(role: str, *, project_id: str | None) -> UUID: """Add one valid local administrative grant through the fixture grantor.""" actor_id, _, grantor_id = await ensure_access_administrator_bootstrap() async with db_session.get_session_factory()() as session: @@ -1959,7 +1950,9 @@ async def test_project_create_exact_replay_and_mismatch_are_atomic( assert project is not None event = await session.get(AuditEvent, project.authorization_decision_event_id) allowed_event_count = await session.scalar( - select(func.count()).select_from(AuditEvent).where( + select(func.count()) + .select_from(AuditEvent) + .where( AuditEvent.action_id == "project.create", AuditEvent.event_type == "SensitiveAuthorizationAllowed", AuditEvent.target_ref_id == project.id, @@ -2006,8 +1999,7 @@ async def test_project_create_concurrent_exact_replay_commits_once( select(func.count()) .select_from(ProjectCreateIdempotencyRecord) .where( - ProjectCreateIdempotencyRecord.idempotency_key - == UUID(headers["Idempotency-Key"]) + ProjectCreateIdempotencyRecord.idempotency_key == UUID(headers["Idempotency-Key"]) ) ) assert project_count == replay_count == 1 @@ -2027,6 +2019,7 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d assert response.status_code == 201, response.text guide = response.json() async with db_session.get_session_factory()() as session: + review_id = revision_id = None if review_policy is not None: values = ( review_policy @@ -2035,14 +2028,38 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d "requires_second_review": False, "allowed_decisions": ["accept", "needs_revision", "reject"], "minimum_finding_fields": ["issue", "required_fix"], - "sla_hours": 24, } ) + values.pop("sla_hours", None) + review_hash = policy_digest( + "review", + ReviewPolicySemantics.model_validate( + { + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, + "max_active_review_leases_per_reviewer": 1, + "self_review_allowed": False, + "reject_policy": "close_task", + "finding_evidence_requirement": "optional", + **values, + } + ), + ) + review_id = str(uuid4()) session.add( ReviewPolicy( - id=str(uuid4()), + id=review_id, project_id=project_id, guide_version=guide["version"], + policy_generation=1, + policy_hash=review_hash, + semantics_status="complete", + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + max_active_review_leases_per_reviewer=1, + self_review_allowed=False, + reject_policy="close_task", + finding_evidence_requirement="optional", **values, ) ) @@ -2053,16 +2070,23 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d else { "max_revision_rounds": 7, "revision_deadline_hours": 48, - "auto_reject_after_limit": True, "allowed_resubmission_states": ["needs_revision"], "reviewer_reassignment_rule": "same reviewer preferred", } ) + values.pop("auto_reject_after_limit", None) + revision_hash = policy_digest( + "revision", RevisionPolicySemantics.model_validate(values) + ) + revision_id = str(uuid4()) session.add( RevisionPolicy( - id=str(uuid4()), + id=revision_id, project_id=project_id, guide_version=guide["version"], + policy_generation=1, + policy_hash=revision_hash, + semantics_status="complete", **values, ) ) @@ -2087,6 +2111,16 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d **values, ) ) + guide_row = await session.get(ProjectGuide, guide["id"]) + assert guide_row is not None + if review_id is not None: + guide_row.selected_review_policy_id = review_id + guide_row.selected_review_policy_generation = 1 + guide_row.selected_review_policy_hash = review_hash + if revision_id is not None: + guide_row.selected_revision_policy_id = revision_id + guide_row.selected_revision_policy_generation = 1 + guide_row.selected_revision_policy_hash = revision_hash await session.commit() await add_project_manager_admin_grant(project_id) if source_snapshot is not None: @@ -2757,9 +2791,7 @@ async def test_guide_source_metadata_authority_records_exact_provenance_and_repl async with db_session.get_session_factory()() as session: persisted_guide = await session.get(ProjectGuide, guide["id"]) - persisted_snapshot = await session.get( - GuideSourceSnapshot, snapshotted.json()["id"] - ) + persisted_snapshot = await session.get(GuideSourceSnapshot, snapshotted.json()["id"]) records = ( await session.scalars( @@ -2774,19 +2806,13 @@ async def test_guide_source_metadata_authority_records_exact_provenance_and_repl assert persisted_guide.last_mutation_scope_type == "system" assert persisted_guide.last_mutation_scope_project_id is None assert persisted_guide.last_authorization_decision_event_id is not None - assert ( - persisted_snapshot.creation_action_id - == "project.guide_source_snapshot.create" - ) + assert persisted_snapshot.creation_action_id == "project.guide_source_snapshot.create" assert persisted_snapshot.authorization_decision_event_id is not None assert len(records) == 4 assert all(record.status == "committed" for record in records) assert sum(record.action_id == "project.guide.create" for record in records) == 1 assert ( - sum( - record.action_id == "project.guide_source_snapshot.create" - for record in records - ) + sum(record.action_id == "project.guide_source_snapshot.create" for record in records) == 1 ) @@ -2921,17 +2947,25 @@ async def prepared_context(current_request, current_resolved, current_session): assert ( await guide_mutation_router_module.guide_authorization_actor( - key, request, result, session, rate_control # type: ignore[arg-type] + key, + request, + result, + session, + rate_control, # type: ignore[arg-type] ) is resolved ) dependency = guide_mutation_router_module.get_guide_prepared_authorization_service( - request, resolved, session # type: ignore[arg-type] + request, + resolved, + session, # type: ignore[arg-type] ) assert await anext(dependency) is prepared await dependency.aclose() assert await guide_mutation_router_module.guide_authorization( - key, resolved, prepared # type: ignore[arg-type] + key, + resolved, + prepared, # type: ignore[arg-type] ) == (key, resolved, prepared) assert calls == [ (request, result, session, rate_control), @@ -2942,14 +2976,10 @@ async def prepared_context(current_request, current_resolved, current_session): def test_guide_mutation_router_translates_bounded_service_errors() -> None: pending = guide_mutation_router_module._error( - guide_mutation_router_module.GuideMutationIdempotencyConflict( - "idempotency_pending" - ) + guide_mutation_router_module.GuideMutationIdempotencyConflict("idempotency_pending") ) mismatch = guide_mutation_router_module._error( - guide_mutation_router_module.GuideMutationIdempotencyConflict( - "idempotency_mismatch" - ) + guide_mutation_router_module.GuideMutationIdempotencyConflict("idempotency_mismatch") ) missing = guide_mutation_router_module._error(ProjectNotFound("project not found")) @@ -3305,9 +3335,7 @@ def test_guide_mutation_service_classifies_reservation_outcomes() -> None: match="idempotency_pending", ): service._reservation_outcome("pending", record, _GuideMutationTestResponse) - replayed = service._reservation_outcome( - "replayed", record, _GuideMutationTestResponse - ) + replayed = service._reservation_outcome("replayed", record, _GuideMutationTestResponse) assert replayed.response == ("validated", {"id": "response"}) assert replayed.replayed is True @@ -3333,9 +3361,7 @@ def __init__(self): self.denied = None async def prepare(self, *_args): - raise PreparedAuthorizationUnsupported( - AuthorizationDenialCode.PERMISSION_NOT_GRANTED - ) + raise PreparedAuthorizationUnsupported(AuthorizationDenialCode.PERMISSION_NOT_GRANTED) async def deny_unsupported(self, *args): self.denied = args @@ -3455,9 +3481,12 @@ async def test_guide_source_metadata_authority_validates_key_before_actor_provis assert response.status_code == 422 async with db_session.get_session_factory()() as session: - assert await session.scalar( - select(ActorIdentityLink).where(ActorIdentityLink.subject == subject) - ) is None + assert ( + await session.scalar( + select(ActorIdentityLink).where(ActorIdentityLink.subject == subject) + ) + is None + ) async def test_create_guide_source_metadata_concurrent_replay_commits_once( @@ -3483,18 +3512,28 @@ async def test_create_guide_source_metadata_concurrent_replay_commits_once( assert first.status_code == second.status_code == 201 assert first.json() == second.json() async with db_session.get_session_factory()() as session: - assert await session.scalar( - select(func.count()).select_from(ProjectGuide).where( - ProjectGuide.project_id == project["id"], - ProjectGuide.version == payload["version"], + assert ( + await session.scalar( + select(func.count()) + .select_from(ProjectGuide) + .where( + ProjectGuide.project_id == project["id"], + ProjectGuide.version == payload["version"], + ) ) - ) == 1 - assert await session.scalar( - select(func.count()).select_from(GuideMutationIdempotencyRecord).where( - GuideMutationIdempotencyRecord.project_id == project["id"], - GuideMutationIdempotencyRecord.action_id == "project.guide.create", + == 1 + ) + assert ( + await session.scalar( + select(func.count()) + .select_from(GuideMutationIdempotencyRecord) + .where( + GuideMutationIdempotencyRecord.project_id == project["id"], + GuideMutationIdempotencyRecord.action_id == "project.guide.create", + ) ) - ) == 1 + == 1 + ) async def test_guide_source_metadata_authority_enforces_exact_project_scope( @@ -3525,11 +3564,14 @@ async def test_guide_source_metadata_authority_enforces_exact_project_scope( assert guide.last_mutated_by_admin_role_grant_id == grant_id assert guide.last_mutation_scope_type == "project" assert guide.last_mutation_scope_project_id == allowed_project["id"] - assert await session.scalar( - select(func.count()).select_from(ProjectGuide).where( - ProjectGuide.project_id == denied_project["id"] + assert ( + await session.scalar( + select(func.count()) + .select_from(ProjectGuide) + .where(ProjectGuide.project_id == denied_project["id"]) ) - ) == 0 + == 0 + ) denial = await session.scalar( select(AuditEvent).where( AuditEvent.action_id == "project.guide.create", @@ -3588,14 +3630,12 @@ async def test_guide_source_metadata_replay_cannot_cross_project_or_guide( snapshot_headers = auth_headers() | {"Idempotency-Key": snapshot_key} snapshot_body = source_snapshot_payload() first_snapshot = await project_client.post( - f"/api/v1/projects/{first_project['id']}/guides/{first_guide['id']}" - "/source-snapshots", + f"/api/v1/projects/{first_project['id']}/guides/{first_guide['id']}/source-snapshots", headers=snapshot_headers, json=snapshot_body, ) crossed_snapshot = await project_client.post( - f"/api/v1/projects/{second_project['id']}/guides/{second_guide['id']}" - "/source-snapshots", + f"/api/v1/projects/{second_project['id']}/guides/{second_guide['id']}/source-snapshots", headers=snapshot_headers, json=snapshot_body, ) @@ -5838,9 +5878,7 @@ async def test_project_setup_visibility_apis_require_active_local_grant( assert [response.status_code for response in wrong_scope] == [404] * len(endpoints) await revoke_local_admin_role(wrong_scope_grant) - operator_grant = await add_local_admin_role_for_default_actor( - "operator", project_id=None - ) + operator_grant = await add_local_admin_role_for_default_actor("operator", project_id=None) operator = [ await project_client.get(endpoint, headers=auth_headers()) for endpoint in endpoints ] @@ -5854,12 +5892,8 @@ async def test_project_setup_visibility_apis_require_active_local_grant( assert [response.status_code for response in audit] == [200] * len(endpoints) await revoke_local_admin_role(audit_grant) - await add_local_admin_role_for_default_actor( - "finance_authority", project_id=project["id"] - ) - finance = [ - await project_client.get(endpoint, headers=auth_headers()) for endpoint in endpoints - ] + await add_local_admin_role_for_default_actor("finance_authority", project_id=project["id"]) + finance = [await project_client.get(endpoint, headers=auth_headers()) for endpoint in endpoints] assert [response.status_code for response in finance] == [404] * len(endpoints) @@ -9586,8 +9620,7 @@ async def test_post_submit_checker_policy_mutations_still_require_legacy_setup_r approve_post_submit_checker=False, ) diagnostic = await project_client.get( - f"/api/v1/projects/{project['id']}/guides/{guide['id']}/" - "post-submit-checker-policy/setup", + f"/api/v1/projects/{project['id']}/guides/{guide['id']}/post-submit-checker-policy/setup", headers=auth_headers(), ) assert diagnostic.status_code == 200, diagnostic.text @@ -9758,7 +9791,8 @@ async def test_review_policy_rejects_invalid_decision_names(project_client: Asyn "requires_second_review": False, "allowed_decisions": ["accept", "hold"], "minimum_finding_fields": ["issue", "required_fix"], - "sla_hours": 24, + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, } response = await project_client.post( @@ -9801,7 +9835,6 @@ async def test_activation_requires_complete_revision_policy(project_client: Asyn payload["revision_policy"] = { "max_revision_rounds": 7, "revision_deadline_hours": 48, - "auto_reject_after_limit": True, "allowed_resubmission_states": [], "reviewer_reassignment_rule": "same reviewer preferred", } @@ -9820,7 +9853,6 @@ async def test_revision_policy_requires_deadline(project_client: AsyncClient) -> payload = complete_guide_payload() payload["revision_policy"] = { "max_revision_rounds": 7, - "auto_reject_after_limit": True, "allowed_resubmission_states": ["needs_revision"], "reviewer_reassignment_rule": "same reviewer preferred", } @@ -9865,7 +9897,6 @@ async def test_activation_rejects_unsupported_revision_resubmission_states( payload["revision_policy"] = { "max_revision_rounds": 7, "revision_deadline_hours": 48, - "auto_reject_after_limit": True, "allowed_resubmission_states": ["random_state"], "reviewer_reassignment_rule": "same reviewer preferred", } @@ -10154,7 +10185,7 @@ async def test_guide_activation_and_active_guide_retrieval(project_client: Async == (bundle["pre_submit_checker_policy"]["checker_configs"]) ) assert active.json()["revision_policy"]["max_revision_rounds"] == 7 - assert active.json()["revision_policy"]["auto_reject_after_limit"] is True + assert "auto_reject_after_limit" not in active.json()["revision_policy"] async def test_draft_guide_edit_and_active_guide_edit_block(project_client: AsyncClient) -> None: @@ -10340,9 +10371,12 @@ async def test_project_create_requires_valid_idempotency_before_actor_provisioni assert response.status_code == 422 async with db_session.get_session_factory()() as session: - assert await session.scalar( - select(ActorIdentityLink).where(ActorIdentityLink.subject == subject) - ) is None + assert ( + await session.scalar( + select(ActorIdentityLink).where(ActorIdentityLink.subject == subject) + ) + is None + ) async def test_project_create_different_keys_same_slug_rolls_back_authority( @@ -10364,14 +10398,20 @@ async def test_project_create_different_keys_same_slug_rolls_back_authority( assert conflict.json()["error"]["code"] == "project_slug_conflict" async with db_session.get_session_factory()() as session: - assert await session.scalar( - select(func.count()).select_from(Project).where(Project.slug == slug) - ) == 1 - assert await session.scalar( - select(func.count()) - .select_from(ProjectCreateIdempotencyRecord) - .where(ProjectCreateIdempotencyRecord.status == "pending") - ) == 0 + assert ( + await session.scalar( + select(func.count()).select_from(Project).where(Project.slug == slug) + ) + == 1 + ) + assert ( + await session.scalar( + select(func.count()) + .select_from(ProjectCreateIdempotencyRecord) + .where(ProjectCreateIdempotencyRecord.status == "pending") + ) + == 0 + ) async def test_project_create_copied_key_cannot_cross_actor_namespace( @@ -10425,15 +10465,23 @@ async def test_project_create_copied_key_cannot_cross_actor_namespace( assert copied.status_code == 409 assert copied.json()["error"]["code"] == "project_slug_conflict" async with db_session.get_session_factory()() as session: - assert await session.scalar( - select(func.count()).select_from(Project).where(Project.slug == payload["slug"]) - ) == 1 - assert await session.scalar( - select(func.count()).select_from(ProjectCreateIdempotencyRecord).where( - ProjectCreateIdempotencyRecord.idempotency_key == UUID(key), - ProjectCreateIdempotencyRecord.status == "pending", + assert ( + await session.scalar( + select(func.count()).select_from(Project).where(Project.slug == payload["slug"]) + ) + == 1 + ) + assert ( + await session.scalar( + select(func.count()) + .select_from(ProjectCreateIdempotencyRecord) + .where( + ProjectCreateIdempotencyRecord.idempotency_key == UUID(key), + ProjectCreateIdempotencyRecord.status == "pending", + ) ) - ) == 0 + == 0 + ) async def test_project_create_denies_project_scoped_and_contributor_authority( @@ -10462,21 +10510,32 @@ async def test_project_create_denies_project_scoped_and_contributor_authority( assert denied.status_code == 403 async with db_session.get_session_factory()() as session: - assert await session.scalar( - select(func.count()).select_from(Project).where(Project.name == "Wrong scope") - ) == 0 - assert await session.scalar( - select(func.count()).select_from(ProjectCreateIdempotencyRecord).where( - ProjectCreateIdempotencyRecord.status == "pending" + assert ( + await session.scalar( + select(func.count()).select_from(Project).where(Project.name == "Wrong scope") ) - ) == 0 - assert await session.scalar( - select(func.count()).select_from(AuditEvent).where( - AuditEvent.action_id == "project.create", - AuditEvent.event_type == "SensitiveAuthorizationAllowed", - AuditEvent.target_ref_id != seed["id"], + == 0 + ) + assert ( + await session.scalar( + select(func.count()) + .select_from(ProjectCreateIdempotencyRecord) + .where(ProjectCreateIdempotencyRecord.status == "pending") ) - ) == 0 + == 0 + ) + assert ( + await session.scalar( + select(func.count()) + .select_from(AuditEvent) + .where( + AuditEvent.action_id == "project.create", + AuditEvent.event_type == "SensitiveAuthorizationAllowed", + AuditEvent.target_ref_id != seed["id"], + ) + ) + == 0 + ) denial_event = await session.scalar( select(AuditEvent).where( AuditEvent.action_id == "project.create", diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 5e2ced515..787d42910 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -62,6 +62,11 @@ build_project_post_submit_checker_spec, compile_project_post_submit_checker_spec, ) +from app.modules.projects.policy_lineage import ( + ReviewPolicySemantics, + RevisionPolicySemantics, + policy_digest, +) from app.modules.tasks.lifecycle import InvalidTaskTransition, ensure_allowed_transition from app.modules.tasks.models import ( AuditEvent, @@ -957,24 +962,54 @@ async def create_policy_bundle_for_guide( async with db_session.get_session_factory()() as session: guide = await session.get(ProjectGuide, guide_id) assert guide is not None + review_policy_id = str(uuid4()) + revision_policy_id = str(uuid4()) + review_hash = policy_digest( + "review", + ReviewPolicySemantics( + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + allowed_decisions=("accept", "needs_revision", "reject"), + minimum_finding_fields=("issue", "required_fix"), + ), + ) + revision_hash = policy_digest( + "revision", + RevisionPolicySemantics( + max_revision_rounds=7, + revision_deadline_hours=48, + allowed_resubmission_states=("needs_revision",), + reviewer_reassignment_rule="same reviewer preferred", + ), + ) session.add_all( [ ReviewPolicy( - id=str(uuid4()), + id=review_policy_id, project_id=project_id, guide_version=guide.version, + policy_generation=1, + policy_hash=review_hash, + semantics_status="complete", + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + max_active_review_leases_per_reviewer=1, + self_review_allowed=False, + reject_policy="close_task", + finding_evidence_requirement="optional", requires_second_review=False, allowed_decisions=["accept", "needs_revision", "reject"], minimum_finding_fields=["issue", "required_fix"], - sla_hours=24, ), RevisionPolicy( - id=str(uuid4()), + id=revision_policy_id, project_id=project_id, guide_version=guide.version, + policy_generation=1, + policy_hash=revision_hash, + semantics_status="complete", max_revision_rounds=7, revision_deadline_hours=48, - auto_reject_after_limit=True, allowed_resubmission_states=["needs_revision"], reviewer_reassignment_rule="same reviewer preferred", ), @@ -991,6 +1026,13 @@ async def create_policy_bundle_for_guide( ), ] ) + await session.flush() + guide.selected_review_policy_id = review_policy_id + guide.selected_review_policy_generation = 1 + guide.selected_review_policy_hash = review_hash + guide.selected_revision_policy_id = revision_policy_id + guide.selected_revision_policy_generation = 1 + guide.selected_revision_policy_hash = revision_hash await session.commit() snapshot_response = await client.post( @@ -2406,8 +2448,12 @@ async def test_screening_locks_guide_policy_context_and_payment_fields( body = response.json() assert body["status"] == "screening" assert body["locked_guide_version"] == "v1" - assert body["locked_review_policy_version"] == "v1" - assert body["locked_revision_policy_version"] == "v1" + assert body["locked_review_policy_id"] + assert body["locked_review_policy_generation"] == 1 + assert body["locked_review_policy_hash"].startswith("sha256:") + assert body["locked_revision_policy_id"] + assert body["locked_revision_policy_generation"] == 1 + assert body["locked_revision_policy_hash"].startswith("sha256:") assert body["locked_payment_policy_version"] == "v1" assert body["locked_guide_source_snapshot_id"] assert body["locked_guide_source_snapshot_hash"].startswith("sha256:") @@ -2418,7 +2464,31 @@ async def test_screening_locks_guide_policy_context_and_payment_fields( expected_post_submit_policy = await load_post_submit_checker_policy(project["id"]) async with db_session.get_session_factory()() as session: persisted_task = await session.get(WorkstreamTask, task["id"]) + selected_guide = await session.scalar( + select(ProjectGuide).where( + ProjectGuide.project_id == project["id"], ProjectGuide.status == "active" + ) + ) assert persisted_task is not None + assert selected_guide is not None + assert ( + body["locked_review_policy_id"], + body["locked_review_policy_generation"], + body["locked_review_policy_hash"], + ) == ( + selected_guide.selected_review_policy_id, + selected_guide.selected_review_policy_generation, + selected_guide.selected_review_policy_hash, + ) + assert ( + body["locked_revision_policy_id"], + body["locked_revision_policy_generation"], + body["locked_revision_policy_hash"], + ) == ( + selected_guide.selected_revision_policy_id, + selected_guide.selected_revision_policy_generation, + selected_guide.selected_revision_policy_hash, + ) assert persisted_task.locked_post_submit_checker_policy_id == expected_post_submit_policy["id"] assert persisted_task.locked_post_submit_checker_policy_version == "v1" assert ( @@ -3199,8 +3269,12 @@ async def test_full_task_claim_start_flow_writes_audit_events( release_event = next(event for event in events if event["to_status"] == "ready") for event in (screening_event, release_event): assert event["event_payload"]["locked_guide_version"] == "v1" - assert event["event_payload"]["locked_review_policy_version"] == "v1" - assert event["event_payload"]["locked_revision_policy_version"] == "v1" + assert event["event_payload"]["locked_review_policy_id"] + assert event["event_payload"]["locked_review_policy_generation"] == 1 + assert event["event_payload"]["locked_review_policy_hash"].startswith("sha256:") + assert event["event_payload"]["locked_revision_policy_id"] + assert event["event_payload"]["locked_revision_policy_generation"] == 1 + assert event["event_payload"]["locked_revision_policy_hash"].startswith("sha256:") assert event["event_payload"]["locked_payment_policy_version"] == "v1" claim_event = next(event for event in events if event["to_status"] == "claimed") assert claim_event["actor_id"] == worker_actor_id @@ -3758,8 +3832,12 @@ async def test_assigned_worker_submit_auto_enters_pre_review_gate( "artifact_hash_manifest", "worker_attestation", "locked_guide_version", - "locked_review_policy_version", - "locked_revision_policy_version", + "locked_review_policy_id", + "locked_review_policy_generation", + "locked_review_policy_hash", + "locked_revision_policy_id", + "locked_revision_policy_generation", + "locked_revision_policy_hash", "locked_payment_policy_version", "locked_guide_source_snapshot_id", "locked_guide_source_snapshot_hash", @@ -3793,6 +3871,24 @@ async def test_assigned_worker_submit_auto_enters_pre_review_gate( persisted_submission.locked_post_submit_checker_policy_hash == persisted_task.locked_post_submit_checker_policy_hash ) + assert ( + persisted_submission.locked_review_policy_id, + persisted_submission.locked_review_policy_generation, + persisted_submission.locked_review_policy_hash, + ) == ( + persisted_task.locked_review_policy_id, + persisted_task.locked_review_policy_generation, + persisted_task.locked_review_policy_hash, + ) + assert ( + persisted_submission.locked_revision_policy_id, + persisted_submission.locked_revision_policy_generation, + persisted_submission.locked_revision_policy_hash, + ) == ( + persisted_task.locked_revision_policy_id, + persisted_task.locked_revision_policy_generation, + persisted_task.locked_revision_policy_hash, + ) task = await task_client.get(f"/api/v1/tasks/{started_task['id']}", headers=auth_headers()) assert task.status_code == 200, task.text @@ -3894,8 +3990,12 @@ async def test_submission_schema_rejects_worker_supplied_locked_context( "locked_post_submit_checker_policy_version": "malicious", "locked_post_submit_checker_policy_hash": "sha256:" + "0" * 64, "locked_post_submit_checker_policy_body": {"required_checkers": []}, - "locked_review_policy_version": "malicious", - "locked_revision_policy_version": "malicious", + "locked_review_policy_id": "malicious", + "locked_review_policy_generation": 99, + "locked_review_policy_hash": "sha256:" + "1" * 64, + "locked_revision_policy_id": "malicious", + "locked_revision_policy_generation": 99, + "locked_revision_policy_hash": "sha256:" + "2" * 64, "locked_payment_policy_version": "malicious", "locked_guide_source_snapshot_id": "malicious", "locked_guide_source_snapshot_hash": "sha256:" + "0" * 64, @@ -4643,8 +4743,12 @@ async def test_database_rejects_submission_without_post_submit_policy_context( locked_post_submit_checker_policy_version=None, locked_post_submit_checker_policy_hash=None, locked_post_submit_checker_policy_body=None, - locked_review_policy_version=task.locked_review_policy_version, - locked_revision_policy_version=task.locked_revision_policy_version, + locked_review_policy_id=task.locked_review_policy_id, + locked_review_policy_generation=task.locked_review_policy_generation, + locked_review_policy_hash=task.locked_review_policy_hash, + locked_revision_policy_id=task.locked_revision_policy_id, + locked_revision_policy_generation=task.locked_revision_policy_generation, + locked_revision_policy_hash=task.locked_revision_policy_hash, locked_payment_policy_version=task.locked_payment_policy_version, locked_guide_source_snapshot_id=task.locked_guide_source_snapshot_id, locked_guide_source_snapshot_hash=task.locked_guide_source_snapshot_hash, @@ -4699,8 +4803,12 @@ async def test_database_rejects_checker_run_without_post_submit_policy_context( locked_post_submit_checker_policy_version=None, locked_post_submit_checker_policy_hash=None, locked_post_submit_checker_policy_body=None, - locked_review_policy_version=submission.locked_review_policy_version, - locked_revision_policy_version=submission.locked_revision_policy_version, + locked_review_policy_id=submission.locked_review_policy_id, + locked_review_policy_generation=submission.locked_review_policy_generation, + locked_review_policy_hash=submission.locked_review_policy_hash, + locked_revision_policy_id=submission.locked_revision_policy_id, + locked_revision_policy_generation=submission.locked_revision_policy_generation, + locked_revision_policy_hash=submission.locked_revision_policy_hash, locked_payment_policy_version=submission.locked_payment_policy_version, package_hash=submission.package_hash, artifact_hash_manifest=submission.artifact_hash_manifest, @@ -5129,8 +5237,8 @@ async def test_database_blocks_task_locked_context_mutation_after_submission( task = await session.get(WorkstreamTask, started_task["id"]) assert task is not None task.locked_guide_version = "v2" - task.locked_review_policy_version = "v2" - task.locked_revision_policy_version = "v2" + task.locked_review_policy_hash = "sha256:" + "f" * 64 + task.locked_revision_policy_hash = "sha256:" + "e" * 64 task.locked_payment_policy_version = "v2" with pytest.raises(IntegrityError): await session.commit() @@ -6660,8 +6768,12 @@ async def test_database_enforces_unique_submission_version( locked_post_submit_checker_policy_body=( persisted.locked_post_submit_checker_policy_body ), - locked_review_policy_version=persisted.locked_review_policy_version, - locked_revision_policy_version=persisted.locked_revision_policy_version, + locked_review_policy_id=persisted.locked_review_policy_id, + locked_review_policy_generation=persisted.locked_review_policy_generation, + locked_review_policy_hash=persisted.locked_review_policy_hash, + locked_revision_policy_id=persisted.locked_revision_policy_id, + locked_revision_policy_generation=persisted.locked_revision_policy_generation, + locked_revision_policy_hash=persisted.locked_revision_policy_hash, locked_payment_policy_version=persisted.locked_payment_policy_version, ) ) diff --git a/docs/architecture_data_model.md b/docs/architecture_data_model.md index 22d28b0a7..1c2b3437e 100644 --- a/docs/architecture_data_model.md +++ b/docs/architecture_data_model.md @@ -215,6 +215,12 @@ Fields: - `created_at` - `updated_at` - `superseded_at` +- `selected_review_policy_id` +- `selected_review_policy_generation` +- `selected_review_policy_hash` +- `selected_revision_policy_id` +- `selected_revision_policy_generation` +- `selected_revision_policy_hash` The guide is versioned and human-facing. Its persisted body is the project guide material itself, usually markdown or imported source material. The source @@ -230,6 +236,10 @@ approval/effective provenance; superseded rows additionally retain `superseded_at`. The planned 02A migration enforces this shape and immutable chronology before Task stamping consumes it. +Draft guides may have no selected review/revision policy while the authorized +policy writer is unavailable. Active and superseded guides require both exact +identity triples, and PostgreSQL freezes those selections after activation. + Runtime enforcement uses machine-readable policies attached to the guide version. Workstream does not parse guide prose at submission time to decide which artifact checks to run. Project owners provide open-ended setup material and business terms. Workstream @@ -959,9 +969,18 @@ Fields: - `id` - `project_id` - `guide_version` +- `policy_generation` +- `policy_hash` +- `semantics_status`: `complete | legacy_incomplete` +- `supersedes_policy_id` +- `review_preference_window_seconds` +- `review_lease_duration_seconds` +- `max_active_review_leases_per_reviewer`: `1` in v0.1 +- `self_review_allowed`: `false` in v0.1 +- `reject_policy`: `close_task` in v0.1 +- `finding_evidence_requirement` - `allowed_decisions` - `minimum_finding_fields` -- `sla_hours` - `created_at` ## RevisionPolicy @@ -971,6 +990,10 @@ Fields: - `id` - `project_id` - `guide_version` +- `policy_generation` +- `policy_hash` +- `semantics_status`: `complete | legacy_incomplete` +- `supersedes_policy_id` - `max_revision_rounds` - `revision_deadline_hours` - `allowed_resubmission_states` @@ -1117,8 +1140,12 @@ Fields: - `locked_post_submit_checker_policy_version` - `locked_post_submit_checker_policy_hash` - `locked_post_submit_checker_policy_body` -- `locked_review_policy_version` -- `locked_revision_policy_version` +- `locked_review_policy_id` +- `locked_review_policy_generation` +- `locked_review_policy_hash` +- `locked_revision_policy_id` +- `locked_revision_policy_generation` +- `locked_revision_policy_hash` - `source_type` - `source_ref` - `source_payload_hash` @@ -1166,9 +1193,10 @@ The task id points to the locked task contract. That contract includes the exact same-project guide ID/version/activation-sequence triplet, guide source snapshot id/hash, effective project submission artifact policy id/hash, generated project pre-submit checker policy id/bundle hash, -post-submit checker policy id/version/hash, review policy version, revision -policy version, acceptance criteria, derived display summaries, and skill tags. -Contributors submit against the task id; they do not restate policy versions. +post-submit checker policy id/version/hash, exact review and revision policy +id/generation/hash identities, acceptance criteria, derived display summaries, +and skill tags. Contributors submit against the task id; they do not restate +policy identities. Durable post-submit checker execution uses `locked_post_submit_checker_policy_id`, @@ -1228,8 +1256,12 @@ Fields: - `locked_post_submit_checker_policy_version` - `locked_post_submit_checker_policy_hash` - `locked_post_submit_checker_policy_body` -- `locked_review_policy_version` -- `locked_revision_policy_version` +- `locked_review_policy_id` +- `locked_review_policy_generation` +- `locked_review_policy_hash` +- `locked_revision_policy_id` +- `locked_revision_policy_generation` +- `locked_revision_policy_hash` - `submitted_at` - `locked_at` - `supersedes_submission_id` @@ -1257,8 +1289,8 @@ checker, review, and revision policy provenance from trusted task/project state. The contributor does not provide submission version, evidence ids, checker results, checker run ids, guide versions, source snapshots, effective project policy ids/hashes, pre-submit checker ids/bundle hashes, -post-submit checker policy ids/versions/hashes, review policy versions, or -revision policy versions. Submitter award eligibility remains governed by the +post-submit checker policy ids/versions/hashes, exact review policy identities, +or exact revision policy identities. Submitter award eligibility remains governed by the immutable TaskAssignment-frozen `ContributionPolicyVersion` and is not restated on the submission. @@ -1333,8 +1365,12 @@ Fields: - `locked_post_submit_checker_policy_version` - `locked_post_submit_checker_policy_hash` - `locked_post_submit_checker_policy_body` -- `locked_review_policy_version` -- `locked_revision_policy_version` +- `locked_review_policy_id` +- `locked_review_policy_generation` +- `locked_review_policy_hash` +- `locked_revision_policy_id` +- `locked_revision_policy_generation` +- `locked_revision_policy_hash` - `package_hash` - `artifact_hash_manifest` - `artifact_manifest_hash` @@ -1481,7 +1517,9 @@ Fields: - `confidence` - `acceptance_evidence_refs` - `locked_guide_version` -- `locked_review_policy_version` +- `locked_review_policy_id` +- `locked_review_policy_generation` +- `locked_review_policy_hash` - `created_at` - `completed_at` @@ -1559,10 +1597,10 @@ Fields: - `next_locked_effective_project_submission_artifact_policy_hash` - `prior_locked_pre_submit_checker_bundle_hash` - `next_locked_pre_submit_checker_bundle_hash` -- `prior_locked_review_policy_version` -- `next_locked_review_policy_version` -- `prior_locked_revision_policy_version` -- `next_locked_revision_policy_version` +- `prior_locked_review_policy_id`, generation, and hash +- `next_locked_review_policy_id`, generation, and hash +- `prior_locked_revision_policy_id`, generation, and hash +- `next_locked_revision_policy_id`, generation, and hash - `outcome`: `kept | rebased | blocked` - `direction`: `forward | backward | null` - `context_digest` diff --git a/docs/glossary.md b/docs/glossary.md index d3fc685c4..c6bd10986 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -259,7 +259,8 @@ rules, hash algorithm, size limits, and attestation terms before submission. The permission-scoped Project Manager, Operator, or Audit projection of a task's locked guide and policy provenance, including guide source snapshot id/hash, effective policy id/hash, pre-submit checker policy id/hash, post-submit checker -policy id/hash/body summary, and review and revision policy versions. +policy id/hash/body summary, and exact review and revision policy +id/generation/hash identities. ## Task Contract diff --git a/docs/spec_chunk_5_submission_packet_foundation.md b/docs/spec_chunk_5_submission_packet_foundation.md index 538be1867..7bb07008f 100644 --- a/docs/spec_chunk_5_submission_packet_foundation.md +++ b/docs/spec_chunk_5_submission_packet_foundation.md @@ -61,8 +61,12 @@ Chunk 5 stores package and evidence references. Actual file storage still belong - `locked_post_submit_checker_policy_version` - `locked_post_submit_checker_policy_hash` - `locked_post_submit_checker_policy_body` -- `locked_review_policy_version` -- `locked_revision_policy_version` +- `locked_review_policy_id` +- `locked_review_policy_generation` +- `locked_review_policy_hash` +- `locked_revision_policy_id` +- `locked_revision_policy_generation` +- `locked_revision_policy_hash` - `locked_payment_policy_version` - `submitted_at` - `locked_at` diff --git a/docs/spec_chunk_6_checker_contract_records.md b/docs/spec_chunk_6_checker_contract_records.md index b2831668a..84e19b057 100644 --- a/docs/spec_chunk_6_checker_contract_records.md +++ b/docs/spec_chunk_6_checker_contract_records.md @@ -57,8 +57,12 @@ This chunk does not run the full checker framework yet. It defines the durable p - `is_current_for_submission` - `locked_guide_version` - `locked_post_submit_checker_policy_version` -- `locked_review_policy_version` -- `locked_revision_policy_version` +- `locked_review_policy_id` +- `locked_review_policy_generation` +- `locked_review_policy_hash` +- `locked_revision_policy_id` +- `locked_revision_policy_generation` +- `locked_revision_policy_hash` - `locked_payment_policy_version` - `package_hash` - `artifact_hash_manifest` @@ -156,11 +160,11 @@ The run snapshots: - deterministic `artifact_manifest_hash` - locked project guide version - locked post-submit checker policy version -- locked review policy version -- locked revision policy version +- locked review policy id, generation, and hash +- locked revision policy id, generation, and hash - locked payment policy version -Post-submit checker runs must be created only from a loaded, finalized submission. The service must copy `task_id`, `submission_version`, `package_hash`, `artifact_hash_manifest`, `artifact_manifest_hash`, and locked policy versions from that submission. The client does not provide locked guide or policy versions for checker runs. +Post-submit checker runs must be created only from a loaded, finalized submission. The service must copy `task_id`, `submission_version`, `package_hash`, `artifact_hash_manifest`, `artifact_manifest_hash`, and exact locked policy identities from that submission. The client does not provide locked guide or policy identities for checker runs. The migration must add enough constraints or service-level tests to prove a checker run cannot bind `submission_id` to a different task, submission version, or package hash. Prefer a composite foreign key or unique binding where practical; otherwise add explicit service validation and integration tests that fail on mismatched context. diff --git a/docs/spec_review_lifecycle.md b/docs/spec_review_lifecycle.md index 4af6e36a3..39e3f1b61 100644 --- a/docs/spec_review_lifecycle.md +++ b/docs/spec_review_lifecycle.md @@ -18,8 +18,9 @@ capability, contribution participant, or frontend. The canonical REV-AUTH action custody is `.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md`. REV owns lifecycle and immutable policy semantics; AUTH owns evaluation, PREP, -and decision evidence. ReviewPolicy and RevisionPolicy use one future -append-only project-policy writer from XINT-003-02. XINT-002-07A alone activates +and decision evidence. ReviewPolicy and RevisionPolicy use immutable, +append-only identities installed by XINT-003-02A; their only future writer is +the PREP-bound mutation surface owned by XINT-003-02B. XINT-002-07A alone activates the ART evidence-binding ActionId for finding slots; 07B only extends its evaluator to response slots after an exact human revision obligation exists. @@ -205,6 +206,23 @@ no-self-review, finding/evidence, and decision rules. `RevisionPolicy` locks revision limit and deadline inputs. Task execution context remains separate from contribution terms. +Each policy version has its own opaque ID, positive generation, canonical +SHA-256 digest, and exact Project Guide lineage. The Project Guide selects one +review-policy identity and one revision-policy identity. A Task copies both +exact identity triples when it enters screening; every Submission and +CheckerRun then copies and foreign-key chains those same triples through the +Task. Guide version identifies guide lineage only and is never used as a policy +version alias. + +Rows migrated from the pre-lineage schema are retained as +`legacy_incomplete`. They remain readable for historical explanation but cannot +satisfy readiness or activate future review behavior because no lease or +preference semantics are invented from the removed `sla_hours` field. Policy +rows reject update, delete, and truncate at the database boundary. The removed +`auto_reject_after_limit` value is not lifecycle authority: reaching a revision +limit or deadline blocks preparation and never auto-rejects or auto-closes a +Task. + The submitter `ContributionPolicyVersion` freezes on the exact TaskAssignment. The reviewer version freezes independently on each ReviewLease. Project Guide rebase changes neither freeze. A later lease may freeze the then-current diff --git a/docs/template_project_guide.md b/docs/template_project_guide.md index 9b80e28de..12d3e36e6 100644 --- a/docs/template_project_guide.md +++ b/docs/template_project_guide.md @@ -142,6 +142,13 @@ before guide activation. ## Review Policy +- review preference window seconds (positive): +- review lease duration seconds (positive): +- maximum active review leases per reviewer: `1` (fixed in v0.1) +- self review allowed: `false` (fixed in v0.1) +- reject policy: `close_task` (fixed in v0.1) +- finding evidence requirement: `optional | required_for_blocking | required_for_all` + Allowed decisions: - accept @@ -174,17 +181,13 @@ Define: - maximum revision rounds: - revision deadline hours: - allowed resubmission states: -- `RevisionPolicyInput.auto_reject_after_limit`: `false` (required explicitly - on project create/update; the backend schema default is not the v0.1 REV - contract) - limit/deadline exhaustion behavior: block preparation and submission pending reason-bound covered-manager closure; never synthesize reject - reviewer reassignment rule: -Revision-policy activation and task screening must reject an effective policy -whose `auto_reject_after_limit` value is not `false`. That runtime enforcement -belongs to `WS-REV-001-02`; until it activates, this template is a required -configuration precondition and does not claim the guard is available. +Revision-policy activation and task screening require positive limit and +deadline values. Exhaustion blocks further preparation and submission; it never +auto-rejects, auto-closes, or fabricates a human review decision. ## Acceptance Policy diff --git a/examples/terminal_benchmark/terminal_benchmark_api_e2e.py b/examples/terminal_benchmark/terminal_benchmark_api_e2e.py index 869e03f9c..755ea3333 100644 --- a/examples/terminal_benchmark/terminal_benchmark_api_e2e.py +++ b/examples/terminal_benchmark/terminal_benchmark_api_e2e.py @@ -469,15 +469,15 @@ def guide_payload(fixture: TerminalBenchmarkFixture, run_id: str) -> dict: ), "change_summary": "Initial Terminal Benchmark real-world guide", "review_policy": { + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, "requires_second_review": False, "allowed_decisions": ["accept", "needs_revision", "reject"], "minimum_finding_fields": ["issue", "required_fix"], - "sla_hours": 24, }, "revision_policy": { "max_revision_rounds": 7, "revision_deadline_hours": 48, - "auto_reject_after_limit": True, "allowed_resubmission_states": ["needs_revision"], "reviewer_reassignment_rule": "same reviewer preferred", }, @@ -813,15 +813,19 @@ async def create_started_terminal_benchmark_task( manager_token, ) ensure( - { - locked_context["locked_post_submit_checker_policy_version"], - locked_context["locked_review_policy_version"], - locked_context["locked_revision_policy_version"], - locked_context["locked_payment_policy_version"], - } - == {"v1"}, - "screening did not stamp every policy version", + locked_context["locked_post_submit_checker_policy_version"] == "v1", + "screening did not stamp the checker policy version", + ) + ensure(locked_context["locked_review_policy_id"], "review policy id missing") + ensure(locked_context["locked_review_policy_generation"] == 1, "review generation drifted") + ensure(locked_context["locked_review_policy_hash"], "review policy hash missing") + ensure(locked_context["locked_revision_policy_id"], "revision policy id missing") + ensure( + locked_context["locked_revision_policy_generation"] == 1, + "revision generation drifted", ) + ensure(locked_context["locked_revision_policy_hash"], "revision policy hash missing") + ensure(locked_context["locked_payment_policy_version"] == "v1", "payment policy drifted") await request_json( client, "POST", From a189fa0afb8a411ff6349986994b38333e6c590e Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sat, 1 Aug 2026 17:01:05 +0100 Subject: [PATCH 3/7] ci(test): assign policy lineage lane custody --- .../chunks/WS-XINT-003-02A-policy-identity-lineage.md | 1 + .../reviews/WS-XINT-003-02A-internal-review.md | 7 +++++++ .../reviews/WS-XINT-003-02A-pr-trust-bundle.md | 2 ++ backend/scripts/run_test_lanes.py | 1 + 4 files changed, 11 insertions(+) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md index d1440fe5f..9ca87b62b 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md @@ -43,6 +43,7 @@ backend/tests/test_artifact_admission.py backend/tests/test_policy_identity_lineage.py backend/tests/conftest.py backend/scripts/api_contract_e2e.py +backend/scripts/run_test_lanes.py backend/scripts/week2_api_e2e.py examples/terminal_benchmark/terminal_benchmark_api_e2e.py docs/spec_review_lifecycle.md diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md index 0c0c3385c..76769a003 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md @@ -42,3 +42,10 @@ No blocking finding remains. All reviewer sessions completed. The repository-wide 78-percent coverage suite remains assigned to hosted GitHub Actions on the exact PR head. + +The first hosted run failed closed during inventory collection because the new +test module had no semantic-lane custody. The module was assigned to +`shared_foundations`; all 31 CI lane-contract tests pass without changing the +four-lane design or any coverage/failure gate. A local repository collection +then reached unrelated missing Pillow dependencies, so exact collection is +left to the hosted environment that supplies the locked CI dependencies. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md index c8a67705b..33f8f0a1e 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md @@ -37,6 +37,8 @@ unavailable. `git diff --check`: passed. - Hosted GitHub Actions retains the full-suite 78-percent gate and the existing targeted 90-percent subsystem gate; no threshold or failure behavior changed. +- The new policy-lineage test module has explicit `shared_foundations` semantic- + lane custody; the initial fail-closed missing-inventory result was corrected. ## Review diff --git a/backend/scripts/run_test_lanes.py b/backend/scripts/run_test_lanes.py index 7eeddf750..b7387a54e 100644 --- a/backend/scripts/run_test_lanes.py +++ b/backend/scripts/run_test_lanes.py @@ -126,6 +126,7 @@ class TestLane: "tests/test_artifact_recovery.py", "tests/test_db_session.py", "tests/test_outbox.py", + "tests/test_policy_identity_lineage.py", ), ), TestLane( From c35a0150a170fc11b45b3ff6817d3ebb6db2b910 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sat, 1 Aug 2026 17:22:24 +0100 Subject: [PATCH 4/7] test(policy): align regressions with immutable lineage --- .../WS-XINT-003-02A-internal-review.md | 7 ++++ .../WS-XINT-003-02A-pr-trust-bundle.md | 2 ++ backend/app/modules/projects/service.py | 8 +++-- backend/tests/test_artifact_admission.py | 34 ++++--------------- backend/tests/test_projects.py | 7 ++-- backend/tests/test_tasks.py | 20 +---------- 6 files changed, 25 insertions(+), 53 deletions(-) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md index 76769a003..ec3e2d7d9 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md @@ -49,3 +49,10 @@ test module had no semantic-lane custody. The module was assigned to four-lane design or any coverage/failure gate. A local repository collection then reached unrelated missing Pillow dependencies, so exact collection is left to the hosted environment that supplies the locked CI dependencies. + +The next hosted lane run exposed three stale test assumptions: partial guide +selection, mutation of now-immutable policy rows, and a cross-Task CheckerRun +rewrite that the new FK correctly rejects. The tests now prove all-or-none +selection, retain stamped work-context proof through the still-mutable payment +policy, and expect the cross-Task rewrite to fail at the database boundary. +All four corrected focused cases pass in isolated databases. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md index 33f8f0a1e..00cabffce 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md @@ -33,6 +33,8 @@ unavailable. - Focused migration and lineage tests: 10 passed, 74 deselected. - Policy-lineage branch coverage: 9 passed, 100 percent. - Direct isolated cross-Task CheckerRun mismatch: 1 passed. +- Hosted-lane regression corrections for activation, immutable work-context, + and checker admission: 4 focused cases passed in isolated databases. - Ruff, Python compilation, stale contract/wording scans, Markdown links, and `git diff --check`: passed. - Hosted GitHub Actions retains the full-suite 78-percent gate and the existing diff --git a/backend/app/modules/projects/service.py b/backend/app/modules/projects/service.py index 8cedb53ac..56e5456bc 100644 --- a/backend/app/modules/projects/service.py +++ b/backend/app/modules/projects/service.py @@ -3595,10 +3595,12 @@ def validate_activation_ready( raise GuideActivationBlocked( "post-submit checker policy references unregistered checker" ) from exc - if review_policy is None or not review_policy.allowed_decisions: + if review_policy is None or revision_policy is None: + raise GuideActivationBlocked( + "complete review and revision policy selections are required" + ) + if not review_policy.allowed_decisions: raise GuideActivationBlocked("review policy with allowed decisions is required") - if revision_policy is None: - raise GuideActivationBlocked("revision policy is required") try: require_complete_policy( kind="review", diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index e01a380e3..7ce2e321b 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -16,7 +16,7 @@ from alembic.config import Config import pytest from sqlalchemy import func, select, text -from sqlalchemy.exc import DBAPIError +from sqlalchemy.exc import DBAPIError, IntegrityError from sqlalchemy.ext.asyncio import ( # type: ignore[import-not-found] async_sessionmaker, create_async_engine, @@ -484,7 +484,7 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: id=guide_id, project_id=project_id, version=guide_version, - status="active", + status="draft", content_markdown="# Checker guide", approved_by="setup-actor", effective_at=now, @@ -632,7 +632,7 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: async with suspend_historical_product_custody( session, table="project_guides", - triggers=("guide_mutation_product_custody",), + triggers=("guide_mutation_product_custody", "guide_lineage_lifecycle_guard"), ): guide = await session.get(ProjectGuide, guide_id) assert guide is not None @@ -642,6 +642,7 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: guide.selected_revision_policy_id = revision_policy_id guide.selected_revision_policy_generation = 1 guide.selected_revision_policy_hash = revision_hash + guide.status = "active" await session.flush() session.add( WorkstreamTask( @@ -4242,7 +4243,9 @@ async def test_checker_output_requires_exact_active_fixed_service_identity( checker_run = await session.get(CheckerRun, checker_run_id) assert checker_run is not None checker_run.task_id = unrelated_task_id - await session.commit() + with pytest.raises(IntegrityError): + await session.commit() + await session.rollback() async with minted_source(tmp_path / "scratch-source", b"checker") as source: service = ArtifactAdmissionService(session, settings, namespace) @@ -4265,29 +4268,6 @@ async def test_checker_output_requires_exact_active_fixed_service_identity( assert await _count(session, ArtifactPutAttempt) == 0 await session.rollback() - with pytest.raises( - ArtifactAdmissionRelationshipError, - match="checker run relationship is unavailable", - ): - await service.admit( - CheckerOutputArtifactAdmissionRequest( - authorization_context=context, - checker_run_id=UUID(checker_run_id), - logical_role="platform-review", - source=source, - ) - ) - assert await _count(session, ArtifactStorageNamespace) == 0 - assert await _count(session, ArtifactAdmissionScope) == 0 - assert await _count(session, ArtifactAdmissionCharge) == 0 - assert await _count(session, ArtifactPutAttempt) == 0 - await session.rollback() - - checker_run = await session.get(CheckerRun, checker_run_id) - assert checker_run is not None - checker_run.task_id = task_id - await session.commit() - request = CheckerOutputArtifactAdmissionRequest( authorization_context=context, checker_run_id=UUID(checker_run_id), diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index 4cc7b0450..386c41f01 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -2113,11 +2113,10 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d ) guide_row = await session.get(ProjectGuide, guide["id"]) assert guide_row is not None - if review_id is not None: + if review_id is not None and revision_id is not None: guide_row.selected_review_policy_id = review_id guide_row.selected_review_policy_generation = 1 guide_row.selected_review_policy_hash = review_hash - if revision_id is not None: guide_row.selected_revision_policy_id = revision_id guide_row.selected_revision_policy_generation = 1 guide_row.selected_revision_policy_hash = revision_hash @@ -9747,7 +9746,7 @@ async def test_activation_requires_review_policy(project_client: AsyncClient) -> ) assert response.status_code == 422 - assert "review policy" in response.json()["detail"] + assert "review and revision policy selections" in response.json()["detail"] async def test_activation_requires_payment_policy(project_client: AsyncClient) -> None: @@ -9781,7 +9780,7 @@ async def test_activation_requires_revision_policy(project_client: AsyncClient) ) assert response.status_code == 422 - assert "revision policy is required" in response.json()["detail"] + assert "review and revision policy selections" in response.json()["detail"] async def test_review_policy_rejects_invalid_decision_names(project_client: AsyncClient) -> None: diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 787d42910..0c8545bce 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -2778,7 +2778,7 @@ async def test_ready_worker_work_context_omits_private_task_source_fields( assert private_field not in body["task"] -async def test_work_context_uses_stamped_policy_values_after_same_version_policy_mutation( +async def test_work_context_uses_stamped_policy_values_after_payment_policy_mutation( task_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -2792,31 +2792,13 @@ async def test_work_context_uses_stamped_policy_values_after_same_version_policy before = before_response.json() async with db_session.get_session_factory()() as session: - review_policy = await session.scalar( - select(ReviewPolicy).where( - ReviewPolicy.project_id == project["id"], - ReviewPolicy.guide_version == "v1", - ) - ) - revision_policy = await session.scalar( - select(RevisionPolicy).where( - RevisionPolicy.project_id == project["id"], - RevisionPolicy.guide_version == "v1", - ) - ) payment_policy = await session.scalar( select(PaymentPolicy).where( PaymentPolicy.project_id == project["id"], PaymentPolicy.guide_version == "v1", ) ) - assert review_policy is not None - assert revision_policy is not None assert payment_policy is not None - review_policy.requires_second_review = True - review_policy.allowed_decisions = ["reject"] - revision_policy.max_revision_rounds = 1 - revision_policy.revision_deadline_hours = 1 payment_policy.base_amount = Decimal("999.00") payment_policy.currency = "EUR" payment_policy.payout_type = "manual" From 1272900052ecac3dda31488a84041d3dbd76e955 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sat, 1 Aug 2026 17:34:32 +0100 Subject: [PATCH 5/7] test(artifact): preserve guide lock invariant --- .../reviews/WS-XINT-003-02A-internal-review.md | 4 +++- .../reviews/WS-XINT-003-02A-pr-trust-bundle.md | 2 +- backend/tests/test_artifact_admission.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md index ec3e2d7d9..e47ecef5d 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md @@ -55,4 +55,6 @@ selection, mutation of now-immutable policy rows, and a cross-Task CheckerRun rewrite that the new FK correctly rejects. The tests now prove all-or-none selection, retain stamped work-context proof through the still-mutable payment policy, and expect the cross-Task rewrite to fail at the database boundary. -All four corrected focused cases pass in isolated databases. +All five corrected focused cases pass in isolated databases. The fifth retains +the guide-lineage lock test by making a rolled-back non-draft transition that +does not violate the newly required active-policy selection invariant. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md index 00cabffce..98c5b2f79 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md @@ -34,7 +34,7 @@ unavailable. - Policy-lineage branch coverage: 9 passed, 100 percent. - Direct isolated cross-Task CheckerRun mismatch: 1 passed. - Hosted-lane regression corrections for activation, immutable work-context, - and checker admission: 4 focused cases passed in isolated databases. + and checker admission: 5 focused cases passed in isolated databases. - Ruff, Python compilation, stale contract/wording scans, Markdown links, and `git diff --check`: passed. - Hosted GitHub Actions retains the full-suite 78-percent gate and the existing diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index 7ce2e321b..4a380966a 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -3491,7 +3491,7 @@ async def test_guide_admission_facts_lock_snapshot_and_item( ), ): await assertion_session.execute( - text("update project_guides set status = 'active' where id = :guide_id"), + text("update project_guides set status = 'inactive' where id = :guide_id"), {"guide_id": facts.guide_id}, ) await assertion_session.flush() From 133f997611322b4f0adbe180a81332fc69e39da0 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sat, 1 Aug 2026 18:23:51 +0100 Subject: [PATCH 6/7] fix(policy): resolve external lineage review --- .../REVIEW_LOG.md | 6 ++ ...WS-XINT-003-02A-policy-identity-lineage.md | 1 + ...S-XINT-003-02A-external-review-response.md | 40 ++++++++++++ .../WS-XINT-003-02A-pr-trust-bundle.md | 6 ++ backend/app/modules/projects/models.py | 10 +++ backend/app/modules/projects/repository.py | 4 +- backend/tests/test_projects.py | 63 ++++++++++++++----- 7 files changed, 111 insertions(+), 19 deletions(-) create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md index 4b649c11b..d2744836e 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md @@ -74,3 +74,9 @@ CheckerRun-to-Submission Task binding, stale active E2E/docs surfaces, vacuous legacy assertions, asymmetric immutability proof, and the final schema fingerprint. All tracks passed after correction; no finding remains open. The final evidence is in `reviews/WS-XINT-003-02A-internal-review.md`. + +CodeRabbit's PR #242 review found missing ORM identity-shape metadata, overly +broad joined row locks, invalid active-guide fixture ordering, and a duplicated +test semantics mapping. All were valid and corrected. Incremental CodeRabbit +attempts were then rate-limited; the exact response is in +`reviews/WS-XINT-003-02A-external-review-response.md`. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md index 9ca87b62b..92ffcfe73 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02A-policy-identity-lineage.md @@ -64,6 +64,7 @@ docs/spec_chunk_6_checker_contract_records.md .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-preimplementation-review.md .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-internal-review.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md ``` diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md new file mode 100644 index 000000000..e186a211f --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md @@ -0,0 +1,40 @@ +# External Review Response: WS-XINT-003-02A + +## Comments addressed + +- CodeRabbit's active-guide fixture finding was valid. The fixture now creates + a draft, persists both exact selections, and activates in the final guarded + step; the hosted regression subsequently passed. +- CodeRabbit's policy-hash finding was valid as ORM metadata drift. Review and + revision model metadata now mirror migration 0046's existing identity-shape + checks for positive generation, canonical SHA-256 digest, and closed semantics + status. No duplicate database constraint was introduced. +- CodeRabbit's row-lock finding was valid. Joined review/revision lookups now + lock only the selected policy row through `FOR UPDATE OF`, avoiding needless + ProjectGuide contention. +- CodeRabbit's test-helper duplication finding was valid. One merged semantics + mapping now feeds both digest validation and persistence, preventing duplicate + keyword failure and digest/row drift. + +## Comments deferred + +None. The generated description/docstring warnings were stale advisory output: +the PR uses the repository trust bundle and hosted docstring coverage passed. + +## Human decisions needed + +None beyond the repository-required human merge decision. + +## Commands rerun + +- Ruff for application, tests, and scripts. +- Focused policy-lineage, project activation, lock-scope/helper, and artifact + fixture tests. +- Schema fingerprint and migration checks as part of exact-head hosted Backend. +- Authorization/artifact/wording/review-contract/Markdown-link checks. + +## Remaining risks + +CodeRabbit rate-limited incremental reviews after its complete first-head +review. Exact-head Agent Gates and Backend must remain green; the external +comments are verified against and resolved in the current diff. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md index 98c5b2f79..0ddfcec5e 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md @@ -54,6 +54,12 @@ and re-reviewed. GitHub Actions and CodeRabbit must review the exact final PR head. Every valid comment or failing check must be resolved before human merge. +CodeRabbit's complete first-head review produced four valid points across three +inline comments: policy identity metadata, scoped row locking, fixture +activation ordering, and shared test semantics. All are fixed and recorded in +`WS-XINT-003-02A-external-review-response.md`. Later incremental attempts were +rate-limited; exact-head Agent Gates and Backend are the authoritative checks. + ## Remaining risk and follow-up This chunk establishes identity and lineage but deliberately activates no diff --git a/backend/app/modules/projects/models.py b/backend/app/modules/projects/models.py index bd0190254..37004eea3 100644 --- a/backend/app/modules/projects/models.py +++ b/backend/app/modules/projects/models.py @@ -435,6 +435,11 @@ class ReviewPolicy(Base): "policy_hash", name="uq_review_policy_scoped_lineage", ), + CheckConstraint( + "policy_generation > 0 and policy_hash ~ '^sha256:[0-9a-f]{64}$' and " + "semantics_status in ('complete','legacy_incomplete')", + name="review_policy_identity_shape", + ), ) id: Mapped[str] = mapped_column(String(36), primary_key=True) @@ -483,6 +488,11 @@ class RevisionPolicy(Base): "policy_hash", name="uq_revision_policy_scoped_lineage", ), + CheckConstraint( + "policy_generation > 0 and policy_hash ~ '^sha256:[0-9a-f]{64}$' and " + "semantics_status in ('complete','legacy_incomplete')", + name="revision_policy_identity_shape", + ), ) id: Mapped[str] = mapped_column(String(36), primary_key=True) diff --git a/backend/app/modules/projects/repository.py b/backend/app/modules/projects/repository.py index 5ef29f521..ba75d4174 100644 --- a/backend/app/modules/projects/repository.py +++ b/backend/app/modules/projects/repository.py @@ -839,7 +839,7 @@ async def lock_review_policy(self, project_id: str, guide_version: str) -> Revie ProjectGuide.project_id == project_id, ProjectGuide.version == guide_version, ) - .with_for_update() + .with_for_update(of=ReviewPolicy) ) async def lock_revision_policy( @@ -863,7 +863,7 @@ async def lock_revision_policy( ProjectGuide.project_id == project_id, ProjectGuide.version == guide_version, ) - .with_for_update() + .with_for_update(of=RevisionPolicy) ) async def get_pre_submit_checker_policy( diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index 386c41f01..eb1244171 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -148,6 +148,42 @@ async def require(self, action_id: ActionId, resource: Any) -> None: self.calls.append((action_id, resource)) +@pytest.mark.asyncio +async def test_project_policy_lock_queries_lock_only_policy_rows() -> None: + statements: list[Any] = [] + + class Session: + async def scalar(self, statement: Any) -> None: + statements.append(statement) + return None + + repository = ProjectRepository(cast(Any, Session())) + await repository.lock_review_policy("project-id", "v1") + await repository.lock_revision_policy("project-id", "v1") + + rendered = [ + str(statement.compile(dialect=postgresql.dialect())) for statement in statements + ] + assert "FOR UPDATE OF review_policies" in rendered[0] + assert "FOR UPDATE OF project_guides" not in rendered[0] + assert "FOR UPDATE OF revision_policies" in rendered[1] + assert "FOR UPDATE OF project_guides" not in rendered[1] + + +def test_policy_identity_shape_metadata_matches_migration_contract() -> None: + expected = { + "ck_review_policies_review_policy_identity_shape": ReviewPolicy, + "ck_revision_policies_revision_policy_identity_shape": RevisionPolicy, + } + for name, model in expected.items(): + constraint = next(item for item in model.__table__.constraints if item.name == name) + sql = str(constraint.sqltext) + assert "policy_generation > 0" in sql + assert "^sha256:[0-9a-f]{64}$" in sql + assert "complete" in sql + assert "legacy_incomplete" in sql + + class _DiagnosticRepository: def __init__(self, *, project_id: str, guide_id: str, target: Any) -> None: self.project = types.SimpleNamespace(id=project_id) @@ -2031,19 +2067,18 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d } ) values.pop("sla_hours", None) + values = { + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, + "max_active_review_leases_per_reviewer": 1, + "self_review_allowed": False, + "reject_policy": "close_task", + "finding_evidence_requirement": "optional", + **values, + } review_hash = policy_digest( "review", - ReviewPolicySemantics.model_validate( - { - "review_preference_window_seconds": 3600, - "review_lease_duration_seconds": 1800, - "max_active_review_leases_per_reviewer": 1, - "self_review_allowed": False, - "reject_policy": "close_task", - "finding_evidence_requirement": "optional", - **values, - } - ), + ReviewPolicySemantics.model_validate(values), ) review_id = str(uuid4()) session.add( @@ -2054,12 +2089,6 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d policy_generation=1, policy_hash=review_hash, semantics_status="complete", - review_preference_window_seconds=3600, - review_lease_duration_seconds=1800, - max_active_review_leases_per_reviewer=1, - self_review_allowed=False, - reject_policy="close_task", - finding_evidence_requirement="optional", **values, ) ) From e7c438cd349f1181c7a3c2f6c4a277d70df9459c Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 10:06:11 +0100 Subject: [PATCH 7/7] test: guarantee migration cleanup after downgrade failure --- .../WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md | 5 +++-- .../WS-XINT-003-02A-external-review-response.md | 6 ++++-- .../reviews/WS-XINT-003-02A-pr-trust-bundle.md | 10 +++++----- backend/tests/test_guide_bindings.py | 8 +++++--- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md index d2744836e..e08ebeff3 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md @@ -77,6 +77,7 @@ final evidence is in `reviews/WS-XINT-003-02A-internal-review.md`. CodeRabbit's PR #242 review found missing ORM identity-shape metadata, overly broad joined row locks, invalid active-guide fixture ordering, and a duplicated -test semantics mapping. All were valid and corrected. Incremental CodeRabbit -attempts were then rate-limited; the exact response is in +test semantics mapping. Its post-main-merge review also found that the guide +sufficiency migration test did not protect cleanup when the initial downgrade +failed. All findings were valid and corrected; the exact response is in `reviews/WS-XINT-003-02A-external-review-response.md`. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md index e186a211f..660b149dc 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-external-review-response.md @@ -15,6 +15,9 @@ - CodeRabbit's test-helper duplication finding was valid. One merged semantics mapping now feeds both digest validation and persistence, preventing duplicate keyword failure and digest/row drift. +- CodeRabbit's post-merge migration-test cleanup finding was valid. The initial + downgrade now runs inside the protected cleanup block, and engine disposal is + guarded so a partial downgrade failure still attempts restoration to head. ## Comments deferred @@ -35,6 +38,5 @@ None beyond the repository-required human merge decision. ## Remaining risks -CodeRabbit rate-limited incremental reviews after its complete first-head -review. Exact-head Agent Gates and Backend must remain green; the external +Exact-head Agent Gates, Backend, and CodeRabbit must remain green; the external comments are verified against and resolved in the current diff. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md index 0ddfcec5e..201e054c2 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02A-pr-trust-bundle.md @@ -54,11 +54,11 @@ and re-reviewed. GitHub Actions and CodeRabbit must review the exact final PR head. Every valid comment or failing check must be resolved before human merge. -CodeRabbit's complete first-head review produced four valid points across three -inline comments: policy identity metadata, scoped row locking, fixture -activation ordering, and shared test semantics. All are fixed and recorded in -`WS-XINT-003-02A-external-review-response.md`. Later incremental attempts were -rate-limited; exact-head Agent Gates and Backend are the authoritative checks. +CodeRabbit's reviews produced five valid points: policy identity metadata, +scoped row locking, fixture activation ordering, shared test semantics, and +post-merge migration-test cleanup. All are fixed and recorded in +`WS-XINT-003-02A-external-review-response.md`. Exact-head Agent Gates, Backend, +and CodeRabbit remain required external checks. ## Remaining risk and follow-up diff --git a/backend/tests/test_guide_bindings.py b/backend/tests/test_guide_bindings.py index 3a3fdb8c8..52c1ac6d6 100644 --- a/backend/tests/test_guide_bindings.py +++ b/backend/tests/test_guide_bindings.py @@ -132,9 +132,10 @@ async def test_guide_sufficiency_provenance_migration_round_trip( config = Config(str(project_root / "alembic.ini")) config.set_main_option("script_location", str(project_root / "alembic")) with migration_lock(): - await asyncio.to_thread(command.downgrade, config, "0045_guide_metadata_authority") - engine = create_async_engine(isolated_database_env) + engine = None try: + await asyncio.to_thread(command.downgrade, config, "0045_guide_metadata_authority") + engine = create_async_engine(isolated_database_env) async with engine.connect() as connection: absent = await connection.scalar( text("select to_regclass('guide_sufficiency_report_source_usages')") @@ -180,7 +181,8 @@ async def test_guide_sufficiency_provenance_migration_round_trip( assert "error_artifact_incident_id" in setup_columns finally: await asyncio.to_thread(command.upgrade, config, "head") - await engine.dispose() + if engine is not None: + await engine.dispose() @pytest.mark.asyncio