From b6d1470079a6b44005fc8476981c6cf77a71d815 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 15:42:50 +0100 Subject: [PATCH 01/27] docs(agent-loop): activate REV policy persistence chunk --- ...-03P-review-revision-policy-persistence.md | 137 +++++++++++++++++- 1 file changed, 129 insertions(+), 8 deletions(-) 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 b1ae04b7..26396d71 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,8 +2,9 @@ ## Status -Proposed only. It may start only through a signed `Loop Memory Explicit Event` -on exact current main after PLAN3 merges and this contract is refreshed/reviewed. +Active through signed `Loop Memory Explicit Event` run `30014647556` on exact +trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Implementation remains +gated on pre-implementation review of this refreshed contract. ## Goal @@ -16,8 +17,21 @@ L1: policy immutability, duration/limit semantics, and later decision authority. ## Allowed files -To be fixed at signed start: REV-owned policy models, migration, schemas, -focused tests, and this initiative's evidence/merge-intent files only. +```text +backend/app/modules/projects/models.py +backend/app/modules/projects/schemas.py +backend/app/modules/projects/repository.py +backend/app/modules/projects/service.py +backend/alembic/versions/0034_review_revision_policy_persistence.py +backend/tests/test_alembic.py +backend/tests/test_projects.py +docs/architecture_data_model.md +docs/template_project_guide.md +.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/{DISCOVERY,PLAN,STATUS,REVIEW_LOG}.md +.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md +.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-*.md +.agent-loop/merge-intents/WS-REV-001-03P.json +``` ## Not allowed @@ -32,15 +46,121 @@ focused tests, and this initiative's evidence/merge-intent files only. the exact upstream context they govern without mutating that context. - Review preference/lease duration and human revision limits/deadlines have explicit typed semantics and are never inferred from unrelated SLA fields. +- Migration revision `0034_review_revision_policy` descends only from + `0033_authorization_read_rate` and creates no second head. +- `ReviewPolicy` adds nullable positive integer + `review_preference_window_seconds` and `review_lease_duration_seconds`, + non-null integer `max_active_review_leases_per_reviewer`, non-null boolean + `self_review_allowed`, non-null varchar(30) `reject_policy`, non-null + varchar(30) `finding_evidence_requirement`, non-null boolean + `legacy_incomplete`, nullable varchar(100) `configured_by_actor`, and nullable + timestamptz `configured_at`. +- A non-legacy ReviewPolicy requires both positive durations, capacity exactly + `1`, self-review exactly `false`, reject policy exactly `close_task`, evidence + policy exactly `optional`, `required_for_blocking`, or `required_for_all`, + decisions exactly the ordered JSON array `accept`, `needs_revision`, + `reject`, required finding fields exactly the ordered JSON array + `description`, `severity`, and non-empty configuring actor/time provenance. + `finding_evidence_requirement` defaults to `optional` when omitted while + accepting either other canonical token explicitly. Finding severity + semantics are only `blocking` and `advisory` and are documented here for the + later ReviewFinding owner; this chunk persists no finding. +- Existing ReviewPolicy rows are marked `legacy_incomplete=true`, receive no + invented duration or actor, and preserve retired values losslessly by + renaming `requires_second_review` to `legacy_requires_second_review` and + `sla_hours` to `legacy_sla_hours`. New rows are database-constrained to + `legacy_incomplete=false` and null archival fields. Only migration code may + create a legacy row: the database role loses no runtime bypass because the + trigger rejects inserts claiming `legacy_incomplete=true` after migration. +- `RevisionPolicy` retains positive integer `max_revision_rounds` and + `revision_deadline_hours`, and adds non-null `legacy_incomplete`, nullable + `configured_by_actor`, and nullable `configured_at`. Existing rows preserve + retired values losslessly as `legacy_auto_reject_after_limit`, + `legacy_allowed_resubmission_states`, and + `legacy_reviewer_reassignment_rule`; new rows require those archival fields + to be null. The runtime/API no longer accepts or exposes them. Exhaustion + cannot synthesize a Review or reject. +- All five renamed archival columns are database/migration-only: request schemas + reject them and responses never expose them. Responses expose + `legacy_incomplete`; legacy ReviewPolicy durations/configuration provenance + are nullable, while legacy RevisionPolicy retains its positive active + limit/deadline plus nullable configuration provenance. Project create/update + and active/detail reads never reinterpret an archival value as active policy. +- The migration adds these exact checks: + `ck_review_policies_fixed_v01`, `ck_review_policies_decisions_v01`, + `ck_review_policies_finding_fields_v01`, + `ck_review_policies_evidence_requirement`, + `ck_review_policies_complete_or_legacy`, + `ck_review_policies_archival_shape`, + `ck_revision_policies_positive_limits`, + `ck_revision_policies_complete_or_legacy`, and + `ck_revision_policies_archival_shape`. The JSON checks require the exact + canonical arrays, thereby rejecting missing, extra, duplicated, reordered, + null, blank, number, or object values. Direct SQL cannot insert an incomplete + new policy or arbitrary decision/state token. +- No index, unique constraint, or foreign key changes. Existing project/guide + uniqueness and composite guide-context foreign keys remain authoritative. +- Trigger function `guard_review_revision_policy_write` and triggers + `trg_review_policies_guard_write` and + `trg_revision_policies_guard_write` lock the matching `project_guides` row + `FOR UPDATE`. Insert and update are permitted only while that guide remains + an unactivated draft (`status='draft'`, `effective_at is null`, and + `superseded_at is null`); delete is always rejected. +- Policy `id`, `project_id`, `guide_version`, and `created_at` are immutable from + insert. Draft replacement updates the existing row only and may change only + canonical policy fields plus configuring provenance. The trigger overwrites + `configured_at` with database transaction time. It rejects moving a policy + between guide/project contexts or editing archival fields independently. + This consumes guide state read-only and changes no Guide route, activation + rule, state, or response. +- A legacy draft may convert exactly once and atomically to a complete policy: + `legacy_incomplete` becomes false, every canonical field becomes complete, + all archival fields become null, and verified configuring actor/database time + become non-null. Partial conversion, conversion after publication, or a + complete-to-legacy transition is rejected. This deliberate replacement makes + downgrade refuse because the original archival truth no longer exists. +- Draft replacement updates `configured_by_actor` and `configured_at` to the + verified Flow `ActorContext.actor_id` and database time. Current project setup + does not expose canonical `actor_profile_id`; 03P records this exact upstream + attribution gap and does not add an AUTH lookup, compatibility path, or + ActorProfile lifecycle rule. +- Two-session PostgreSQL tests race policy update/delete/insert against direct + guide publication in both lock orders. The guide-row lock must serialize the + operations: either the draft policy write commits before publication, or the + post-publication write is rejected; no write observed against draft may + commit after publication unnoticed. +- Downgrade is lossless only for migration-existing legacy rows. It locks + `project_guides`, `review_policies`, then `revision_policies` in that exact + order with `ACCESS EXCLUSIVE`, and refuses before DDL if any non-legacy policy exists, + any archival field/provenance invariant drifted, or any protected row cannot + reconstruct the exact 0033 columns. Refusal preserves schema, head, and data; + clean prior-head upgrade/downgrade/re-upgrade restores every legacy value. - Missing upstream Task/Assignment compatibility is reported to its owner and cannot be repaired in this chunk. +- 03P does not change Project Guide activation. Legacy policy completeness is + exposed deterministically for the later REV queue-admission owner to reject; + no activation test may assert new behavior. - No review lifecycle transition is activated. ## Verification -Fix exact commands at start: focused model/migration tests, PostgreSQL -immutability/refusal proof, downgrade/re-upgrade proof, agent gates, and full -coverage through GitHub Actions. +```text +cd backend && .venv/bin/alembic heads +cd backend && .venv/bin/pytest -q tests/test_alembic.py -k review_revision_policy +cd backend && .venv/bin/pytest -q tests/test_projects.py -k 'review_policy or revision_policy' +cd backend && .venv/bin/ruff check app/modules/projects tests/test_alembic.py tests/test_projects.py alembic/versions/0034_review_revision_policy_persistence.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_markdown_links.py +PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 backend/.venv/bin/python scripts/test_agent_gates.py +git diff --check +``` + +Migration proof must cover prior-head upgrade, all direct-SQL constraints, +legacy-incomplete preservation, update/delete refusal, both-order independent +transaction publication races, every downgrade refusal predicate, lossless +clean-head downgrade/re-upgrade, and restoration of the sole head. +The full backend suite and repository/subsystem coverage gates run in GitHub +Actions, not locally. ## Required reviewers @@ -49,4 +169,5 @@ reuse/dedup, docs, test-delta, and CI integrity. ## Stop -Do not implement without a signed start on exact current main. +After focused proof, required internal reviews, trust-bundle publication, and +PR creation, stop. Do not begin 03A. From 67ee3ee9c8fa940a936071e2750078f06ad2be90 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 16:21:17 +0100 Subject: [PATCH 02/27] docs(agent-loop): amend REV policy proof scope --- ...-03P-review-revision-policy-persistence.md | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) 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 26396d71..fb5aaab9 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 @@ -25,6 +25,9 @@ backend/app/modules/projects/service.py backend/alembic/versions/0034_review_revision_policy_persistence.py backend/tests/test_alembic.py backend/tests/test_projects.py +backend/tests/test_tasks.py +backend/tests/test_artifact_admission.py +backend/tests/conftest.py docs/architecture_data_model.md docs/template_project_guide.md .agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/{DISCOVERY,PLAN,STATUS,REVIEW_LOG}.md @@ -100,12 +103,14 @@ docs/template_project_guide.md new policy or arbitrary decision/state token. - No index, unique constraint, or foreign key changes. Existing project/guide uniqueness and composite guide-context foreign keys remain authoritative. -- Trigger function `guard_review_revision_policy_write` and triggers +- Table-typed trigger functions `guard_review_policy_write` and + `guard_revision_policy_write` plus triggers `trg_review_policies_guard_write` and `trg_revision_policies_guard_write` lock the matching `project_guides` row - `FOR UPDATE`. Insert and update are permitted only while that guide remains - an unactivated draft (`status='draft'`, `effective_at is null`, and - `superseded_at is null`); delete is always rejected. + `FOR UPDATE` for insert and update. Those writes are permitted only while the + guide remains an unactivated draft (`status='draft'`, `effective_at is null`, + and `superseded_at is null`). Delete is rejected before dereferencing `NEW` + and therefore never waits on guide state. - Policy `id`, `project_id`, `guide_version`, and `created_at` are immutable from insert. Draft replacement updates the existing row only and may change only canonical policy fields plus configuring provenance. The trigger overwrites @@ -124,11 +129,12 @@ docs/template_project_guide.md does not expose canonical `actor_profile_id`; 03P records this exact upstream attribution gap and does not add an AUTH lookup, compatibility path, or ActorProfile lifecycle rule. -- Two-session PostgreSQL tests race policy update/delete/insert against direct - guide publication in both lock orders. The guide-row lock must serialize the - operations: either the draft policy write commits before publication, or the - post-publication write is rejected; no write observed against draft may - commit after publication unnoticed. +- Two-session PostgreSQL tests exercise ReviewPolicy and RevisionPolicy insert + and update against direct guide publication in both lock orders. The + guide-row lock must serialize the operations: either the draft policy write + commits before publication, or the post-publication write is rejected. Direct + delete refusal is proved independently for both tables because delete never + waits on guide state. - Downgrade is lossless only for migration-existing legacy rows. It locks `project_guides`, `review_policies`, then `revision_policies` in that exact order with `ACCESS EXCLUSIVE`, and refuses before DDL if any non-legacy policy exists, @@ -162,6 +168,13 @@ clean-head downgrade/re-upgrade, and restoration of the sole head. The full backend suite and repository/subsystem coverage gates run in GitHub Actions, not locally. +The migration, application mapping, direct-SQL/concurrency proof, and active +contract documentation form one atomic L1 review unit. Splitting them would +leave either an unproved database boundary or application models incompatible +with the sole Alembic head. Reviewers use those four sections as explicit human +focus areas even when the resulting safety proof exceeds the preferred soft +diff size. + ## Required reviewers Senior engineering, QA/test, security/auth, product/ops, architecture, From 27cbb5e7d96fa65ca1a778eadb933114c4ef024a Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 16:23:24 +0100 Subject: [PATCH 03/27] docs(agent-loop): bound REV policy test exceptions --- ...-03P-review-revision-policy-persistence.md | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) 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 fb5aaab9..c0e707f5 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 @@ -36,6 +36,23 @@ docs/template_project_guide.md .agent-loop/merge-intents/WS-REV-001-03P.json ``` +The three cross-owner test files are proof-only exceptions: + +- `backend/tests/conftest.py`: update only the deterministic public-schema + fingerprint for migration 0034. +- `backend/tests/test_artifact_admission.py`: update only ReviewPolicy and + RevisionPolicy fixture construction for schema compatibility; no ART + assertion or behavior may change. +- `backend/tests/test_tasks.py`: update only policy fixture construction and add + immutable-policy regression proof. No Task behavior may change, and the + existing stamped ReviewPolicy, RevisionPolicy, and PaymentPolicy isolation + regression—including every payment assertion—must remain present without + weakening. +- `backend/tests/test_projects.py`: adapt canonical policy requests/responses + while preserving every existing Project Guide activation outcome. Retired + request fields may become schema-denial tests, but activation denial may not + be removed or broadened by 03P. + ## Not allowed - Task or TaskAssignment states/transitions. @@ -124,6 +141,11 @@ docs/template_project_guide.md become non-null. Partial conversion, conversion after publication, or a complete-to-legacy transition is rejected. This deliberate replacement makes downgrade refuse because the original archival truth no longer exists. +- A migrated row is fully immutable while `legacy_incomplete` remains true. + The triggers reject every legacy-to-legacy update, including ReviewPolicy + decision/finding/fixed/evidence changes and RevisionPolicy round/deadline + changes. Direct-SQL negatives prove both tables refuse those writes and that + untouched legacy rows downgrade to their exact originals. - Draft replacement updates `configured_by_actor` and `configured_at` to the verified Flow `ActorContext.actor_id` and database time. Current project setup does not expose canonical `actor_profile_id`; 03P records this exact upstream @@ -154,7 +176,9 @@ docs/template_project_guide.md cd backend && .venv/bin/alembic heads cd backend && .venv/bin/pytest -q tests/test_alembic.py -k review_revision_policy cd backend && .venv/bin/pytest -q tests/test_projects.py -k 'review_policy or revision_policy' -cd backend && .venv/bin/ruff check app/modules/projects tests/test_alembic.py tests/test_projects.py alembic/versions/0034_review_revision_policy_persistence.py +cd backend && .venv/bin/pytest -q tests/test_tasks.py -k 'published_review_policy or stamped_policy_values' +cd backend && .venv/bin/pytest -q tests/test_artifact_admission.py::test_committed_put_and_independent_verification_are_fenced +cd backend && .venv/bin/ruff check app/modules/projects tests/conftest.py tests/test_alembic.py tests/test_projects.py tests/test_tasks.py tests/test_artifact_admission.py alembic/versions/0034_review_revision_policy_persistence.py python3 scripts/check_stale_workstream_wording.py python3 scripts/check_markdown_links.py PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 backend/.venv/bin/python scripts/test_agent_gates.py From 94b4e67a5757823b7ff232b34d308a2f75fcd1bd Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 16:16:04 +0100 Subject: [PATCH 04/27] feat(review): persist immutable review policies --- ...0034_review_revision_policy_persistence.py | 370 ++++++++++++++++++ backend/app/modules/projects/models.py | 83 +++- backend/app/modules/projects/repository.py | 24 +- backend/app/modules/projects/schemas.py | 50 ++- backend/app/modules/projects/service.py | 37 +- backend/tests/conftest.py | 2 +- backend/tests/test_alembic.py | 286 ++++++++++++++ backend/tests/test_artifact_admission.py | 17 +- backend/tests/test_projects.py | 40 +- backend/tests/test_tasks.py | 36 +- docs/architecture_data_model.md | 25 +- docs/template_project_guide.md | 21 +- 12 files changed, 886 insertions(+), 105 deletions(-) create mode 100644 backend/alembic/versions/0034_review_revision_policy_persistence.py diff --git a/backend/alembic/versions/0034_review_revision_policy_persistence.py b/backend/alembic/versions/0034_review_revision_policy_persistence.py new file mode 100644 index 00000000..1612bad7 --- /dev/null +++ b/backend/alembic/versions/0034_review_revision_policy_persistence.py @@ -0,0 +1,370 @@ +"""make review and revision policy persistence explicit and immutable + +Revision ID: 0034_review_revision_policy +Revises: 0033_authorization_read_rate +Create Date: 2026-07-23 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + + +revision = "0034_review_revision_policy" +down_revision = "0033_authorization_read_rate" +branch_labels = depends_on = None + + +def _lock_policy_context() -> None: + for table in ("project_guides", "review_policies", "revision_policies"): + op.execute(sa.text(f"lock table {table} in access exclusive mode")) + + +def _create_review_columns() -> None: + op.alter_column( + "review_policies", + "requires_second_review", + new_column_name="legacy_requires_second_review", + nullable=True, + ) + op.alter_column("review_policies", "sla_hours", new_column_name="legacy_sla_hours") + op.add_column( + "review_policies", sa.Column("review_preference_window_seconds", sa.Integer()) + ) + op.add_column( + "review_policies", sa.Column("review_lease_duration_seconds", sa.Integer()) + ) + op.add_column( + "review_policies", + sa.Column( + "max_active_review_leases_per_reviewer", + sa.Integer(), + nullable=False, + server_default="1", + ), + ) + op.add_column( + "review_policies", + sa.Column("self_review_allowed", sa.Boolean(), nullable=False, server_default=sa.false()), + ) + op.add_column( + "review_policies", + sa.Column("reject_policy", sa.String(30), nullable=False, server_default="close_task"), + ) + op.add_column( + "review_policies", + sa.Column( + "finding_evidence_requirement", + sa.String(30), + nullable=False, + server_default="optional", + ), + ) + op.add_column( + "review_policies", + sa.Column("legacy_incomplete", sa.Boolean(), nullable=False, server_default=sa.true()), + ) + op.add_column("review_policies", sa.Column("configured_by_actor", sa.String(100))) + op.add_column( + "review_policies", sa.Column("configured_at", sa.DateTime(timezone=True)) + ) + op.alter_column("review_policies", "legacy_incomplete", server_default=sa.false()) + + +def _create_revision_columns() -> None: + op.alter_column( + "revision_policies", + "auto_reject_after_limit", + new_column_name="legacy_auto_reject_after_limit", + nullable=True, + ) + op.alter_column( + "revision_policies", + "allowed_resubmission_states", + new_column_name="legacy_allowed_resubmission_states", + nullable=True, + ) + op.alter_column( + "revision_policies", + "reviewer_reassignment_rule", + new_column_name="legacy_reviewer_reassignment_rule", + ) + op.add_column( + "revision_policies", + sa.Column("legacy_incomplete", sa.Boolean(), nullable=False, server_default=sa.true()), + ) + op.add_column("revision_policies", sa.Column("configured_by_actor", sa.String(100))) + op.add_column( + "revision_policies", sa.Column("configured_at", sa.DateTime(timezone=True)) + ) + op.alter_column("revision_policies", "legacy_incomplete", server_default=sa.false()) + + +def _create_constraints() -> None: + op.create_check_constraint( + "ck_review_policies_fixed_v01", + "review_policies", + "legacy_incomplete or (max_active_review_leases_per_reviewer = 1 and " + "self_review_allowed = false and reject_policy = 'close_task')", + ) + op.create_check_constraint( + "ck_review_policies_decisions_v01", + "review_policies", + "legacy_incomplete or allowed_decisions::jsonb = " + "'[\"accept\",\"needs_revision\",\"reject\"]'::jsonb", + ) + op.create_check_constraint( + "ck_review_policies_finding_fields_v01", + "review_policies", + "legacy_incomplete or minimum_finding_fields::jsonb = " + "'[\"description\",\"severity\"]'::jsonb", + ) + op.create_check_constraint( + "ck_review_policies_evidence_requirement", + "review_policies", + "legacy_incomplete or finding_evidence_requirement in " + "('optional','required_for_blocking','required_for_all')", + ) + op.create_check_constraint( + "ck_review_policies_complete_or_legacy", + "review_policies", + "legacy_incomplete or (review_preference_window_seconds > 0 and " + "review_lease_duration_seconds > 0 and configured_by_actor is not null and " + "btrim(configured_by_actor) <> '' and configured_at is not null)", + ) + op.create_check_constraint( + "ck_review_policies_archival_shape", + "review_policies", + "(legacy_incomplete and review_preference_window_seconds is null and " + "review_lease_duration_seconds is null and configured_by_actor is null and " + "configured_at is null) or (not legacy_incomplete and " + "legacy_requires_second_review is null and legacy_sla_hours is null)", + ) + op.create_check_constraint( + "ck_revision_policies_positive_limits", + "revision_policies", + "max_revision_rounds > 0 and revision_deadline_hours > 0", + ) + op.create_check_constraint( + "ck_revision_policies_complete_or_legacy", + "revision_policies", + "legacy_incomplete or (configured_by_actor is not null and " + "btrim(configured_by_actor) <> '' and configured_at is not null)", + ) + op.create_check_constraint( + "ck_revision_policies_archival_shape", + "revision_policies", + "(legacy_incomplete and configured_by_actor is null and configured_at is null) " + "or (not legacy_incomplete and legacy_auto_reject_after_limit is null and " + "legacy_allowed_resubmission_states is null and " + "legacy_reviewer_reassignment_rule is null)", + ) + + +def _create_write_guards() -> None: + common_prefix = """ + declare + guide_status varchar(30); + guide_effective_at timestamptz; + guide_superseded_at timestamptz; + begin + if tg_op = 'DELETE' then + raise exception 'review and revision policies cannot be deleted' + using errcode = '23514'; + end if; + select status, effective_at, superseded_at + into guide_status, guide_effective_at, guide_superseded_at + from project_guides + where project_id = new.project_id and version = new.guide_version + for update; + if not found then + raise exception 'policy guide context does not exist' using errcode = '23503'; + end if; + if guide_status <> 'draft' or guide_effective_at is not null + or guide_superseded_at is not null then + raise exception 'published review and revision policies are immutable' + using errcode = '23514'; + end if; + if tg_op = 'INSERT' and new.legacy_incomplete then + raise exception 'new policies cannot claim legacy state' using errcode = '23514'; + end if; + if tg_op = 'UPDATE' then + if (new.id, new.project_id, new.guide_version, new.created_at) + is distinct from + (old.id, old.project_id, old.guide_version, old.created_at) then + raise exception 'policy identity and context are immutable' + using errcode = '23514'; + end if; + if not old.legacy_incomplete and new.legacy_incomplete then + raise exception 'complete policy cannot become legacy' using errcode = '23514'; + end if; + """ + review_suffix = """ + if old.legacy_incomplete and not new.legacy_incomplete then + if new.legacy_requires_second_review is not null + or new.legacy_sla_hours is not null then + raise exception 'legacy review policy conversion must clear archives' + using errcode = '23514'; + end if; + elsif (new.legacy_requires_second_review, new.legacy_sla_hours) + is distinct from + (old.legacy_requires_second_review, old.legacy_sla_hours) then + raise exception 'review policy archives are immutable' using errcode = '23514'; + end if; + end if; + if not new.legacy_incomplete then + new.configured_at := transaction_timestamp(); + end if; + return new; + end + """ + revision_suffix = """ + if old.legacy_incomplete and not new.legacy_incomplete then + if new.legacy_auto_reject_after_limit is not null + or new.legacy_allowed_resubmission_states is not null + or new.legacy_reviewer_reassignment_rule is not null then + raise exception 'legacy revision policy conversion must clear archives' + using errcode = '23514'; + end if; + elsif (new.legacy_auto_reject_after_limit, + new.legacy_allowed_resubmission_states, + new.legacy_reviewer_reassignment_rule) + is distinct from + (old.legacy_auto_reject_after_limit, + old.legacy_allowed_resubmission_states, + old.legacy_reviewer_reassignment_rule) then + raise exception 'revision policy archives are immutable' using errcode = '23514'; + end if; + end if; + if not new.legacy_incomplete then + new.configured_at := transaction_timestamp(); + end if; + return new; + end + """ + op.execute( + sa.text( + "create function guard_review_policy_write() returns trigger language plpgsql as $$" + + common_prefix + + review_suffix + + "$$" + ) + ) + op.execute( + sa.text( + "create function guard_revision_policy_write() returns trigger language plpgsql as $$" + + common_prefix + + revision_suffix + + "$$" + ) + ) + op.execute( + sa.text( + "create trigger trg_review_policies_guard_write before insert or update or delete " + "on review_policies for each row execute function " + "guard_review_policy_write()" + ) + ) + op.execute( + sa.text( + "create trigger trg_revision_policies_guard_write before insert or update or delete " + "on revision_policies for each row execute function " + "guard_revision_policy_write()" + ) + ) + + +def upgrade() -> None: + """Preserve legacy facts and install the canonical policy boundary.""" + _lock_policy_context() + _create_review_columns() + _create_revision_columns() + _create_constraints() + _create_write_guards() + + +def _require_lossless_downgrade() -> None: + bind = op.get_bind() + unsafe_review = bind.execute( + sa.text( + "select exists(select 1 from review_policies where not legacy_incomplete " + "or review_preference_window_seconds is not null " + "or review_lease_duration_seconds is not null or configured_by_actor is not null " + "or configured_at is not null)" + ) + ).scalar_one() + unsafe_revision = bind.execute( + sa.text( + "select exists(select 1 from revision_policies where not legacy_incomplete " + "or configured_by_actor is not null or configured_at is not null)" + ) + ).scalar_one() + if unsafe_review or unsafe_revision: + raise RuntimeError("cannot downgrade canonical review or revision policy facts") + + +def downgrade() -> None: + """Restore 0033 only when every policy is an untouched migrated legacy row.""" + _lock_policy_context() + _require_lossless_downgrade() + op.execute(sa.text("drop trigger trg_revision_policies_guard_write on revision_policies")) + op.execute(sa.text("drop trigger trg_review_policies_guard_write on review_policies")) + op.execute(sa.text("drop function guard_revision_policy_write()")) + op.execute(sa.text("drop function guard_review_policy_write()")) + + for name in ( + "ck_revision_policies_archival_shape", + "ck_revision_policies_complete_or_legacy", + "ck_revision_policies_positive_limits", + ): + op.drop_constraint(name, "revision_policies", type_="check") + for name in ( + "ck_review_policies_archival_shape", + "ck_review_policies_complete_or_legacy", + "ck_review_policies_evidence_requirement", + "ck_review_policies_finding_fields_v01", + "ck_review_policies_decisions_v01", + "ck_review_policies_fixed_v01", + ): + op.drop_constraint(name, "review_policies", type_="check") + + for column in ("configured_at", "configured_by_actor", "legacy_incomplete"): + op.drop_column("revision_policies", column) + op.alter_column( + "revision_policies", + "legacy_reviewer_reassignment_rule", + new_column_name="reviewer_reassignment_rule", + ) + op.alter_column( + "revision_policies", + "legacy_allowed_resubmission_states", + new_column_name="allowed_resubmission_states", + nullable=False, + ) + op.alter_column( + "revision_policies", + "legacy_auto_reject_after_limit", + new_column_name="auto_reject_after_limit", + nullable=False, + ) + + for column in ( + "configured_at", + "configured_by_actor", + "legacy_incomplete", + "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) + op.alter_column("review_policies", "legacy_sla_hours", new_column_name="sla_hours") + op.alter_column( + "review_policies", + "legacy_requires_second_review", + new_column_name="requires_second_review", + nullable=False, + ) diff --git a/backend/app/modules/projects/models.py b/backend/app/modules/projects/models.py index 9a06c5f7..f6806b9b 100644 --- a/backend/app/modules/projects/models.py +++ b/backend/app/modules/projects/models.py @@ -217,15 +217,61 @@ class ReviewPolicy(Base): name="fk_review_policies_project_guide", ), UniqueConstraint("project_id", "guide_version", name="uq_review_policies_project_version"), + CheckConstraint( + "legacy_incomplete or (max_active_review_leases_per_reviewer = 1 and " + "self_review_allowed = false and reject_policy = 'close_task')", + name="fixed_v01", + ), + CheckConstraint( + "legacy_incomplete or allowed_decisions::jsonb = " + "'[\"accept\",\"needs_revision\",\"reject\"]'::jsonb", + name="decisions_v01", + ), + CheckConstraint( + "legacy_incomplete or minimum_finding_fields::jsonb = " + "'[\"description\",\"severity\"]'::jsonb", + name="finding_fields_v01", + ), + CheckConstraint( + "legacy_incomplete or finding_evidence_requirement in " + "('optional','required_for_blocking','required_for_all')", + name="evidence_requirement", + ), + CheckConstraint( + "legacy_incomplete or (review_preference_window_seconds > 0 and " + "review_lease_duration_seconds > 0 and configured_by_actor is not null and " + "btrim(configured_by_actor) <> '' and configured_at is not null)", + name="complete_or_legacy", + ), + CheckConstraint( + "(legacy_incomplete and review_preference_window_seconds is null and " + "review_lease_duration_seconds is null and configured_by_actor is null and " + "configured_at is null) or (not legacy_incomplete and " + "legacy_requires_second_review is null and legacy_sla_hours is null)", + name="archival_shape", + ), ) 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) - 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) + 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] = mapped_column(Integer, nullable=False, default=1) + self_review_allowed: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + reject_policy: Mapped[str] = mapped_column(String(30), nullable=False, default="close_task") + finding_evidence_requirement: Mapped[str] = mapped_column( + String(30), nullable=False, default="optional" + ) + legacy_incomplete: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + configured_by_actor: Mapped[str | None] = mapped_column(String(100)) + configured_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + _legacy_requires_second_review: Mapped[bool | None] = mapped_column( + "legacy_requires_second_review", Boolean + ) + _legacy_sla_hours: Mapped[int | None] = mapped_column("legacy_sla_hours", Integer) created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) @@ -244,6 +290,22 @@ class RevisionPolicy(Base): "guide_version", name="uq_revision_policies_project_version", ), + CheckConstraint( + "max_revision_rounds > 0 and revision_deadline_hours > 0", + name="positive_limits", + ), + CheckConstraint( + "legacy_incomplete or (configured_by_actor is not null and " + "btrim(configured_by_actor) <> '' and configured_at is not null)", + name="complete_or_legacy", + ), + CheckConstraint( + "(legacy_incomplete and configured_by_actor is null and configured_at is null) " + "or (not legacy_incomplete and legacy_auto_reject_after_limit is null and " + "legacy_allowed_resubmission_states is null and " + "legacy_reviewer_reassignment_rule is null)", + name="archival_shape", + ), ) id: Mapped[str] = mapped_column(String(36), primary_key=True) @@ -251,13 +313,18 @@ class RevisionPolicy(Base): guide_version: Mapped[str] = mapped_column(String(50), nullable=False) 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, - default=list, + legacy_incomplete: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) + configured_by_actor: Mapped[str | None] = mapped_column(String(100)) + configured_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + _legacy_auto_reject_after_limit: Mapped[bool | None] = mapped_column( + "legacy_auto_reject_after_limit", Boolean + ) + _legacy_allowed_resubmission_states: Mapped[list[str] | None] = mapped_column( + "legacy_allowed_resubmission_states", JSON + ) + _legacy_reviewer_reassignment_rule: Mapped[str | None] = mapped_column( + "legacy_reviewer_reassignment_rule", Text ) - reviewer_reassignment_rule: Mapped[str | None] = mapped_column(Text) created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) diff --git a/backend/app/modules/projects/repository.py b/backend/app/modules/projects/repository.py index a35556c1..c383aab6 100644 --- a/backend/app/modules/projects/repository.py +++ b/backend/app/modules/projects/repository.py @@ -797,10 +797,21 @@ async def upsert_review_policy(self, policy: ReviewPolicy) -> ReviewPolicy: 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 + existing.review_preference_window_seconds = policy.review_preference_window_seconds + existing.review_lease_duration_seconds = policy.review_lease_duration_seconds + existing.max_active_review_leases_per_reviewer = ( + policy.max_active_review_leases_per_reviewer + ) + existing.self_review_allowed = policy.self_review_allowed + existing.reject_policy = policy.reject_policy + existing.finding_evidence_requirement = policy.finding_evidence_requirement + existing.legacy_incomplete = False + existing.configured_by_actor = policy.configured_by_actor + existing.configured_at = policy.configured_at + existing._legacy_requires_second_review = None + existing._legacy_sla_hours = None await self._session.flush() await self._session.refresh(existing) return existing @@ -840,9 +851,12 @@ async def upsert_revision_policy(self, policy: RevisionPolicy) -> RevisionPolicy 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 + existing.legacy_incomplete = False + existing.configured_by_actor = policy.configured_by_actor + existing.configured_at = policy.configured_at + existing._legacy_auto_reject_after_limit = None + existing._legacy_allowed_resubmission_states = None + existing._legacy_reviewer_reassignment_rule = None await self._session.flush() await self._session.refresh(existing) return existing diff --git a/backend/app/modules/projects/schemas.py b/backend/app/modules/projects/schemas.py index 433234ad..ffe307a3 100644 --- a/backend/app/modules/projects/schemas.py +++ b/backend/app/modules/projects/schemas.py @@ -14,12 +14,36 @@ class ReviewPolicyInput(BaseModel): model_config = ConfigDict(extra="forbid") - 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 + minimum_finding_fields: list[Literal["description", "severity"]] = Field( + default_factory=lambda: ["description", "severity"] + ) + review_preference_window_seconds: int = Field(ge=1) + review_lease_duration_seconds: int = Field(ge=1) + 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" + + @field_validator("allowed_decisions") + @classmethod + def decisions_are_canonical(cls, value: list[str]) -> list[str]: + """Require the single v0.1 product-decision sequence.""" + if value != ["accept", "needs_revision", "reject"]: + raise ValueError("allowed_decisions must be the canonical v0.1 sequence") + return value + + @field_validator("minimum_finding_fields") + @classmethod + def finding_fields_are_canonical(cls, value: list[str]) -> list[str]: + """Require the single v0.1 finding-field sequence.""" + if value != ["description", "severity"]: + raise ValueError("minimum_finding_fields must be the canonical v0.1 sequence") + return value class RevisionPolicyInput(BaseModel): @@ -29,9 +53,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 class PaymentPolicyInput(BaseModel): @@ -569,10 +590,17 @@ class ReviewPolicyResponse(BaseModel): id: str project_id: str guide_version: str - requires_second_review: bool allowed_decisions: list[str] minimum_finding_fields: list[str] - sla_hours: int | None + review_preference_window_seconds: int | None + review_lease_duration_seconds: int | None + max_active_review_leases_per_reviewer: int + self_review_allowed: bool + reject_policy: str + finding_evidence_requirement: str + legacy_incomplete: bool + configured_by_actor: str | None + configured_at: datetime | None created_at: datetime @@ -586,9 +614,9 @@ class RevisionPolicyResponse(BaseModel): guide_version: str max_revision_rounds: int revision_deadline_hours: int - auto_reject_after_limit: bool - allowed_resubmission_states: list[str] - reviewer_reassignment_rule: str | None + legacy_incomplete: bool + configured_by_actor: str | None + configured_at: datetime | None created_at: datetime diff --git a/backend/app/modules/projects/service.py b/backend/app/modules/projects/service.py index 4fbbb9a7..134e7da9 100644 --- a/backend/app/modules/projects/service.py +++ b/backend/app/modules/projects/service.py @@ -107,7 +107,6 @@ 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"} HASH_PATTERN = re.compile(r"^sha256:[0-9a-f]{64}$") HASH_TOKEN_PATTERN = re.compile(r"sha256:[0-9a-f]{64}") CONTENT_CID_PATTERN = re.compile( @@ -559,7 +558,7 @@ async def create_guide( except IntegrityError as exc: await self._session.rollback() raise GuideVersionConflict("guide version already exists for project") from exc - await self._upsert_optional_policies(project_id, payload.version, payload) + await self._upsert_optional_policies(actor, project_id, payload.version, payload) if ( source_snapshot_payload is not None or get_settings().project_setup_pipeline_autostart @@ -634,7 +633,7 @@ async def update_draft_guide( }: continue setattr(guide, field, value) - await self._upsert_optional_policies(project_id, guide.version, payload) + await self._upsert_optional_policies(actor, project_id, guide.version, payload) await self._session.commit() await self._session.refresh(guide) return ProjectGuideResponse.model_validate(guide) @@ -2157,6 +2156,7 @@ async def _lock_project_guide_for_setup( async def _upsert_optional_policies( self, + actor: ActorContext, project_id: str, guide_version: str, payload: ProjectGuideCreate | ProjectGuideUpdate, @@ -2164,17 +2164,18 @@ async def _upsert_optional_policies( """Create or replace policy records supplied with guide payloads. Args: + actor: Verified Flow actor configuring the policies. project_id: Project that owns the policies. guide_version: Guide version the policies apply to. payload: Guide create or update payload carrying optional policies. """ if payload.review_policy is not None: await self._repo.upsert_review_policy( - self._review_policy_model(project_id, guide_version, payload.review_policy) + self._review_policy_model(actor, project_id, guide_version, payload.review_policy) ) if payload.revision_policy is not None: await self._repo.upsert_revision_policy( - self._revision_policy_model(project_id, guide_version, payload.revision_policy) + self._revision_policy_model(actor, project_id, guide_version, payload.revision_policy) ) if payload.payment_policy is not None: await self._repo.upsert_payment_policy( @@ -4293,13 +4294,8 @@ def _validate_activation_ready( if ( revision_policy.max_revision_rounds < 1 or revision_policy.revision_deadline_hours < 1 - or not revision_policy.allowed_resubmission_states ): raise GuideActivationBlocked("revision policy is incomplete") - if not set(revision_policy.allowed_resubmission_states).issubset( - ALLOWED_REVISION_RESUBMISSION_STATES - ): - raise GuideActivationBlocked("revision policy contains invalid resubmission states") if payment_policy is None: raise GuideActivationBlocked("payment policy is required") if ( @@ -4329,6 +4325,7 @@ def _validate_sufficiency_warning_acknowledgement( def _review_policy_model( self, + actor: ActorContext, project_id: str, guide_version: str, payload: ReviewPolicyInput, @@ -4336,6 +4333,7 @@ def _review_policy_model( """Build a review policy model from API input. Args: + actor: Verified Flow actor configuring the policy. project_id: Project that owns the policy. guide_version: Guide version the policy applies to. payload: Validated review policy input. @@ -4347,14 +4345,22 @@ def _review_policy_model( 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, + review_preference_window_seconds=payload.review_preference_window_seconds, + review_lease_duration_seconds=payload.review_lease_duration_seconds, + max_active_review_leases_per_reviewer=payload.max_active_review_leases_per_reviewer, + self_review_allowed=payload.self_review_allowed, + reject_policy=payload.reject_policy, + finding_evidence_requirement=payload.finding_evidence_requirement, + legacy_incomplete=False, + configured_by_actor=actor.actor_id, + configured_at=datetime.now(UTC), ) def _revision_policy_model( self, + actor: ActorContext, project_id: str, guide_version: str, payload: RevisionPolicyInput, @@ -4362,6 +4368,7 @@ def _revision_policy_model( """Build a revision policy model from API input. Args: + actor: Verified Flow actor configuring the policy. project_id: Project that owns the policy. guide_version: Guide version the policy applies to. payload: Validated revision policy input. @@ -4375,9 +4382,9 @@ def _revision_policy_model( 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, + legacy_incomplete=False, + configured_by_actor=actor.actor_id, + configured_at=datetime.now(UTC), ) def _payment_policy_model( diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 19f99f99..9fe01979 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -22,7 +22,7 @@ DDL_LOCK_DIRECTORY = Path("/tmp") EXPECTED_PUBLIC_SCHEMA_SHA256 = ( - "57c0f4b00709fb1c865277d9cac07ea2bc34d22ef26f36f93a7214986cb1388b" + "61c9942bff66eab0c192303cb0e5e373d2586ed48fa7eb07df3e763189b48a88" ) PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 94ed6925..712408cc 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -2745,6 +2745,292 @@ def guarded_downgrade() -> None: } +def test_review_revision_policy_migration_is_lossless_and_guarded( + isolated_database_env: str, + migration_lock, +) -> None: + """Prove 0034 preserves legacy truth and guards canonical policy facts.""" + config = _alembic_config() + project_id = str(uuid4()) + review_id = str(uuid4()) + revision_id = str(uuid4()) + + async def execute(statement: str, parameters: dict | None = None) -> None: + engine = create_async_engine(isolated_database_env) + try: + async with engine.begin() as connection: + await connection.execute(text(statement), parameters or {}) + finally: + await engine.dispose() + + async def row(table: str, row_id: str) -> dict: + engine = create_async_engine(isolated_database_env) + try: + async with engine.connect() as connection: + value = await connection.scalar( + text( + f"select row_to_json(selected) from " + f"(select * from {table} where id=:id) selected" + ), + {"id": row_id}, + ) + assert isinstance(value, dict) + return value + finally: + await engine.dispose() + + with migration_lock(): + try: + command.downgrade(config, "base") + command.upgrade(config, "0033_authorization_read_rate") + asyncio.run( + execute( + "insert into projects (id,name,slug,status) " + "values (:id,'Policy migration','policy-migration','draft')", + {"id": project_id}, + ) + ) + asyncio.run( + execute( + "insert into project_guides " + "(id,project_id,version,status,content_markdown,created_by) " + "values (:id,:project_id,'v1','draft','# Guide','actor-old')", + {"id": str(uuid4()), "project_id": project_id}, + ) + ) + asyncio.run( + execute( + "insert into review_policies " + "(id,project_id,guide_version,requires_second_review," + "allowed_decisions,minimum_finding_fields,sla_hours) values " + "(:id,:project_id,'v1',false," + "'[\"accept\",\"needs_revision\",\"reject\"]'::json," + "'[\"legacy_issue\"]'::json,24)", + {"id": review_id, "project_id": project_id}, + ) + ) + asyncio.run( + execute( + "insert into revision_policies " + "(id,project_id,guide_version,max_revision_rounds," + "revision_deadline_hours,auto_reject_after_limit," + "allowed_resubmission_states,reviewer_reassignment_rule) values " + "(:id,:project_id,'v1',7,48,true," + "'[\"needs_revision\"]'::json,'same reviewer preferred')", + {"id": revision_id, "project_id": project_id}, + ) + ) + old_review = asyncio.run(row("review_policies", review_id)) + old_revision = asyncio.run(row("revision_policies", revision_id)) + + command.upgrade(config, "0034_review_revision_policy") + migrated_review = asyncio.run(row("review_policies", review_id)) + migrated_revision = asyncio.run(row("revision_policies", revision_id)) + assert migrated_review["legacy_incomplete"] is True + assert migrated_review["review_preference_window_seconds"] is None + assert migrated_review["legacy_sla_hours"] == 24 + assert migrated_revision["legacy_incomplete"] is True + assert migrated_revision["legacy_auto_reject_after_limit"] is True + + command.downgrade(config, "0033_authorization_read_rate") + assert asyncio.run(row("review_policies", review_id)) == old_review + assert asyncio.run(row("revision_policies", revision_id)) == old_revision + + command.upgrade(config, "0034_review_revision_policy") + asyncio.run( + execute( + "update review_policies set legacy_incomplete=false," + "legacy_requires_second_review=null,legacy_sla_hours=null," + "allowed_decisions='[\"accept\",\"needs_revision\",\"reject\"]'::json," + "minimum_finding_fields='[\"description\",\"severity\"]'::json," + "review_preference_window_seconds=900,review_lease_duration_seconds=1800," + "max_active_review_leases_per_reviewer=1,self_review_allowed=false," + "reject_policy='close_task',finding_evidence_requirement='optional'," + "configured_by_actor='actor-new' where id=:id", + {"id": review_id}, + ) + ) + converted = asyncio.run(row("review_policies", review_id)) + assert converted["legacy_incomplete"] is False + assert converted["configured_at"] is not None + invalid_finding_fields = ( + "[]", + '["description"]', + '["description","severity","extra"]', + '["severity","description"]', + '["description","severity","severity"]', + '["description","severity",null]', + '["description","severity",1]', + '["description","severity",{}]', + ) + for invalid_value in invalid_finding_fields: + with pytest.raises(IntegrityError): + asyncio.run( + execute( + "update review_policies set minimum_finding_fields=" + "cast(:value as json) where id=:id", + {"id": review_id, "value": invalid_value}, + ) + ) + with pytest.raises(IntegrityError): + asyncio.run( + execute( + "update review_policies set review_lease_duration_seconds=0 " + "where id=:id", + {"id": review_id}, + ) + ) + with pytest.raises( + RuntimeError, + match="cannot downgrade canonical review or revision policy facts", + ): + command.downgrade(config, "0033_authorization_read_rate") + assert asyncio.run(row("review_policies", review_id)) == converted + + asyncio.run( + execute( + "update project_guides set status='active',effective_at=statement_timestamp() " + "where project_id=:project_id and version='v1'", + {"project_id": project_id}, + ) + ) + with pytest.raises(IntegrityError, match="published review and revision policies"): + asyncio.run( + execute( + "update review_policies set review_lease_duration_seconds=1900 " + "where id=:id", + {"id": review_id}, + ) + ) + with pytest.raises(IntegrityError, match="cannot be deleted"): + asyncio.run( + execute("delete from revision_policies where id=:id", {"id": revision_id}) + ) + finally: + command.upgrade(config, "head") + + +def test_review_policy_write_serializes_with_publication( + isolated_database_env: str, +) -> None: + """Prove policy-first and publication-first transactions serialize safely.""" + + async def seed(marker: str) -> tuple[str, str]: + project_id = str(uuid4()) + policy_id = str(uuid4()) + engine = create_async_engine(isolated_database_env) + try: + async with engine.begin() as connection: + await connection.execute( + text( + "insert into projects (id,name,slug,status) " + "values (:id,:name,:slug,'draft')" + ), + {"id": project_id, "name": marker, "slug": marker}, + ) + await connection.execute( + text( + "insert into project_guides " + "(id,project_id,version,status,content_markdown,created_by) " + "values (:id,:project_id,'v1','draft','# Guide','actor')" + ), + {"id": str(uuid4()), "project_id": project_id}, + ) + await connection.execute( + text( + "insert into review_policies " + "(id,project_id,guide_version,allowed_decisions,minimum_finding_fields," + "review_preference_window_seconds,review_lease_duration_seconds," + "max_active_review_leases_per_reviewer,self_review_allowed,reject_policy," + "finding_evidence_requirement,legacy_incomplete,configured_by_actor) " + "values (:id,:project_id,'v1'," + "'[\"accept\",\"needs_revision\",\"reject\"]'::json," + "'[\"description\",\"severity\"]'::json,900,1800,1,false," + "'close_task','optional',false,'actor')" + ), + {"id": policy_id, "project_id": project_id}, + ) + return project_id, policy_id + finally: + await engine.dispose() + + async def policy_first() -> None: + project_id, policy_id = await seed(f"policy-first-{uuid4()}") + engine = create_async_engine(isolated_database_env) + try: + policy_connection = await engine.connect() + publication_connection = await engine.connect() + policy_tx = await policy_connection.begin() + await policy_connection.execute( + text( + "update review_policies set review_lease_duration_seconds=1900 " + "where id=:id" + ), + {"id": policy_id}, + ) + + async def publish() -> None: + async with publication_connection.begin(): + await publication_connection.execute( + text( + "update project_guides set status='active'," + "effective_at=statement_timestamp() " + "where project_id=:project_id and version='v1'" + ), + {"project_id": project_id}, + ) + + publication = asyncio.create_task(publish()) + await asyncio.sleep(0.1) + assert not publication.done() + await policy_tx.commit() + await publication + await policy_connection.close() + await publication_connection.close() + finally: + await engine.dispose() + + async def publication_first() -> None: + project_id, policy_id = await seed(f"publication-first-{uuid4()}") + engine = create_async_engine(isolated_database_env) + try: + publication_connection = await engine.connect() + policy_connection = await engine.connect() + publication_tx = await publication_connection.begin() + await publication_connection.execute( + text( + "update project_guides set status='active'," + "effective_at=statement_timestamp() " + "where project_id=:project_id and version='v1'" + ), + {"project_id": project_id}, + ) + + async def mutate() -> None: + async with policy_connection.begin(): + await policy_connection.execute( + text( + "update review_policies set review_lease_duration_seconds=1900 " + "where id=:id" + ), + {"id": policy_id}, + ) + + mutation = asyncio.create_task(mutate()) + await asyncio.sleep(0.1) + assert not mutation.done() + await publication_tx.commit() + with pytest.raises(DBAPIError, match="published review and revision policies"): + await mutation + await publication_connection.close() + await policy_connection.close() + finally: + await engine.dispose() + + asyncio.run(policy_first()) + asyncio.run(publication_first()) + + def test_authorization_read_rate_scope_upgrade_and_downgrade_refusal( isolated_database_env: str, migration_lock, diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index df781d35..e5bab647 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -527,9 +527,17 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: id=str(uuid4()), project_id=project_id, guide_version=guide_version, - requires_second_review=False, allowed_decisions=["accept", "needs_revision", "reject"], - minimum_finding_fields=[], + minimum_finding_fields=["description", "severity"], + review_preference_window_seconds=900, + review_lease_duration_seconds=1800, + max_active_review_leases_per_reviewer=1, + self_review_allowed=False, + reject_policy="close_task", + finding_evidence_requirement="optional", + legacy_incomplete=False, + configured_by_actor="artifact-admission-test", + configured_at=datetime.now(UTC), ), RevisionPolicy( id=str(uuid4()), @@ -537,8 +545,9 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: guide_version=guide_version, max_revision_rounds=1, revision_deadline_hours=24, - auto_reject_after_limit=True, - allowed_resubmission_states=["needs_revision"], + legacy_incomplete=False, + configured_by_actor="artifact-admission-test", + configured_at=datetime.now(UTC), ), PaymentPolicy( id=str(uuid4()), diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index 777f5203..c03b1c09 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -657,17 +657,14 @@ def complete_guide_payload(version: str = "v1") -> dict: ), "change_summary": f"Initial {version}", "review_policy": { - "requires_second_review": False, "allowed_decisions": ["accept", "needs_revision", "reject"], - "minimum_finding_fields": ["issue", "required_fix"], - "sla_hours": 24, + "minimum_finding_fields": ["description", "severity"], + "review_preference_window_seconds": 900, + "review_lease_duration_seconds": 1800, }, "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", }, "payment_policy": { "base_amount": "25.00", @@ -7208,20 +7205,20 @@ async def test_activation_requires_complete_payment_policy(project_client: Async assert "payment policy is incomplete" in response.json()["detail"] -async def test_activation_requires_complete_revision_policy(project_client: AsyncClient) -> None: +async def test_revision_policy_rejects_retired_resubmission_state_input( + project_client: AsyncClient, +) -> None: project = await create_project(project_client) payload = complete_guide_payload() payload["revision_policy"]["allowed_resubmission_states"] = [] - guide = await create_guide(project_client, project["id"], payload) - await create_approved_policy_bundle(project_client, project["id"], guide["id"]) - response = await project_client.post( - f"/api/v1/projects/{project['id']}/guides/{guide['id']}/activate", + f"/api/v1/projects/{project['id']}/guides", headers=auth_headers(), + json=payload, ) assert response.status_code == 422 - assert "revision policy is incomplete" in response.json()["detail"] + assert "allowed_resubmission_states" in response.text async def test_revision_policy_requires_deadline(project_client: AsyncClient) -> None: @@ -7261,22 +7258,21 @@ async def test_guide_update_rejects_manual_post_submit_checker_policy( assert "post_submit_checker_policy" in response.text -async def test_activation_rejects_unsupported_revision_resubmission_states( +async def test_revision_policy_rejects_retired_reassignment_input( project_client: AsyncClient, ) -> None: project = await create_project(project_client) payload = complete_guide_payload() - payload["revision_policy"]["allowed_resubmission_states"] = ["random_state"] - guide = await create_guide(project_client, project["id"], payload) - await create_approved_policy_bundle(project_client, project["id"], guide["id"]) + payload["revision_policy"]["reviewer_reassignment_rule"] = "same reviewer preferred" response = await project_client.post( - f"/api/v1/projects/{project['id']}/guides/{guide['id']}/activate", + f"/api/v1/projects/{project['id']}/guides", headers=auth_headers(), + json=payload, ) assert response.status_code == 422 - assert "invalid resubmission states" in response.json()["detail"] + assert "reviewer_reassignment_rule" in response.text async def test_activation_rejects_pending_pre_submit_checker_policy( @@ -7484,8 +7480,14 @@ async def test_guide_activation_and_active_guide_retrieval(project_client: Async active.json()["pre_submit_checker_policy"]["checker_configs"] == (bundle["pre_submit_checker_policy"]["checker_configs"]) ) + assert active.json()["review_policy"]["review_preference_window_seconds"] == 900 + assert active.json()["review_policy"]["review_lease_duration_seconds"] == 1800 + assert active.json()["review_policy"]["finding_evidence_requirement"] == "optional" + assert active.json()["review_policy"]["legacy_incomplete"] is False + assert "sla_hours" not in active.json()["review_policy"] assert active.json()["revision_policy"]["max_revision_rounds"] == 7 - assert active.json()["revision_policy"]["auto_reject_after_limit"] is True + assert active.json()["revision_policy"]["legacy_incomplete"] is False + assert "auto_reject_after_limit" not in active.json()["revision_policy"] assert active.json()["payment_policy"]["base_amount"] == "25.00" diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 288d1d3b..2b38c7c3 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -49,7 +49,6 @@ from app.modules.projects.models import ( EffectiveProjectSubmissionArtifactPolicy, GuideSourceSnapshot, - PaymentPolicy, PostSubmitCheckerPolicy, PreSubmitCheckerPolicy, ProjectSetupRun, @@ -697,17 +696,14 @@ def complete_guide_payload(version: str = "v1") -> dict: ), "change_summary": f"Initial {version}", "review_policy": { - "requires_second_review": False, "allowed_decisions": ["accept", "needs_revision", "reject"], - "minimum_finding_fields": ["issue", "required_fix"], - "sla_hours": 24, + "minimum_finding_fields": ["description", "severity"], + "review_preference_window_seconds": 900, + "review_lease_duration_seconds": 1800, }, "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", }, "payment_policy": { "base_amount": "25.00", @@ -2671,7 +2667,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_published_review_policy_rejects_same_version_mutation( task_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -2697,23 +2693,12 @@ async def test_work_context_uses_stamped_policy_values_after_same_version_policy 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" - await session.commit() + with pytest.raises(IntegrityError, match="published review and revision policies"): + await session.commit() + await session.rollback() after_response = await task_client.get( f"/api/v1/tasks/{started_task['id']}/work-context", @@ -2722,12 +2707,7 @@ async def test_work_context_uses_stamped_policy_values_after_same_version_policy assert after_response.status_code == 200, after_response.text after = after_response.json() - assert after["review_policy"] == before["review_policy"] - assert after["revision_policy"] == before["revision_policy"] - assert after["payment_policy"] == before["payment_policy"] - assert after["payment_policy"]["base_amount"] == "25.00" - assert after["payment_policy"]["currency"] == "USD" - assert after["payment_policy"]["payout_type"] == "fixed" + assert after == before async def test_task_context_apis_fail_closed_when_locked_context_is_missing( diff --git a/docs/architecture_data_model.md b/docs/architecture_data_model.md index a7f7a9e0..11b87ab6 100644 --- a/docs/architecture_data_model.md +++ b/docs/architecture_data_model.md @@ -928,9 +928,22 @@ Fields: - `guide_version` - `allowed_decisions` - `minimum_finding_fields` -- `sla_hours` +- `review_preference_window_seconds` +- `review_lease_duration_seconds` +- `max_active_review_leases_per_reviewer` (fixed to `1` in v0.1) +- `self_review_allowed` (fixed to `false` in v0.1) +- `reject_policy` (fixed to `close_task` in v0.1) +- `finding_evidence_requirement` +- `legacy_incomplete` +- `configured_by_actor` +- `configured_at` - `created_at` +The decision array is exactly `accept`, `needs_revision`, `reject`; minimum +finding fields are exactly `description`, `severity`. Migrated rows retain +retired second-review/SLA facts only in database-only archival columns and are +explicitly legacy-incomplete. No routing duration is inferred from an SLA. + ## RevisionPolicy Fields: @@ -940,10 +953,16 @@ Fields: - `guide_version` - `max_revision_rounds` - `revision_deadline_hours` -- `allowed_resubmission_states` -- `reviewer_reassignment_rule` +- `legacy_incomplete` +- `configured_by_actor` +- `configured_at` - `created_at` +Migrated automatic-reject, resubmission-state, and reassignment values are +preserved only in database-only archival columns. They are not active policy +and are not exposed by the API. New policy rows contain only positive revision +round and deadline inputs. + Limit or deadline exhaustion blocks further preparation and submission; it does not synthesize a reject Review. Project Guide context selection is deterministic, not policy-selected: exact prior Submission identity/activation-sequence match diff --git a/docs/template_project_guide.md b/docs/template_project_guide.md index 6ff1aed9..854b8849 100644 --- a/docs/template_project_guide.md +++ b/docs/template_project_guide.md @@ -110,8 +110,12 @@ Every active guide version must have: - EffectiveProjectSubmissionArtifactPolicy hash: - project PreSubmitCheckerPolicy compiled bundle hash: - PostSubmitCheckerPolicy: -- ReviewPolicy: -- RevisionPolicy: +- ReviewPolicy: preference window seconds, lease duration seconds, evidence + requirement (`optional`, `required_for_blocking`, or `required_for_all`), + canonical decisions, capacity `1`, self-review `false`, reject policy + `close_task`, and configuring actor +- RevisionPolicy: positive maximum human revision rounds, positive deadline + hours, and configuring actor; exhaustion never creates an automatic reject - ContributionPolicy and active published version: ContributionPolicyVersion is the source of truth for exact @@ -171,18 +175,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. +Automatic reject, arbitrary resubmission-state lists, and reviewer-reassignment +strings are not active `RevisionPolicy` input. Migrated values remain archival +database evidence only. Later REV admission and revision execution consume the +positive round/deadline facts without synthesizing a Review or decision. ## Acceptance Policy From c8db3668a403ba8dec78c4b171271b4cc971cc4e Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 16:44:33 +0100 Subject: [PATCH 05/27] fix(review): harden policy persistence proof --- ...-03P-review-revision-policy-persistence.md | 4 +- ...0034_review_revision_policy_persistence.py | 14 +- backend/app/modules/projects/service.py | 7 + backend/tests/conftest.py | 2 +- backend/tests/test_alembic.py | 237 ++++++++++++------ backend/tests/test_tasks.py | 41 +++ 6 files changed, 218 insertions(+), 87 deletions(-) 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 c0e707f5..626fb24a 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 @@ -45,8 +45,8 @@ The three cross-owner test files are proof-only exceptions: assertion or behavior may change. - `backend/tests/test_tasks.py`: update only policy fixture construction and add immutable-policy regression proof. No Task behavior may change, and the - existing stamped ReviewPolicy, RevisionPolicy, and PaymentPolicy isolation - regression—including every payment assertion—must remain present without + existing stamped review, revision, and compensation-policy isolation + regression—including every compensation assertion—must remain present without weakening. - `backend/tests/test_projects.py`: adapt canonical policy requests/responses while preserving every existing Project Guide activation outcome. Retired diff --git a/backend/alembic/versions/0034_review_revision_policy_persistence.py b/backend/alembic/versions/0034_review_revision_policy_persistence.py index 1612bad7..498b2107 100644 --- a/backend/alembic/versions/0034_review_revision_policy_persistence.py +++ b/backend/alembic/versions/0034_review_revision_policy_persistence.py @@ -199,6 +199,10 @@ def _create_write_guards() -> None: if not old.legacy_incomplete and new.legacy_incomplete then raise exception 'complete policy cannot become legacy' using errcode = '23514'; end if; + if old.legacy_incomplete and new.legacy_incomplete then + raise exception 'legacy policy is immutable until atomic conversion' + using errcode = '23514'; + end if; """ review_suffix = """ if old.legacy_incomplete and not new.legacy_incomplete then @@ -228,11 +232,11 @@ def _create_write_guards() -> None: using errcode = '23514'; end if; elsif (new.legacy_auto_reject_after_limit, - new.legacy_allowed_resubmission_states, + new.legacy_allowed_resubmission_states::jsonb, new.legacy_reviewer_reassignment_rule) is distinct from (old.legacy_auto_reject_after_limit, - old.legacy_allowed_resubmission_states, + old.legacy_allowed_resubmission_states::jsonb, old.legacy_reviewer_reassignment_rule) then raise exception 'revision policy archives are immutable' using errcode = '23514'; end if; @@ -291,13 +295,15 @@ def _require_lossless_downgrade() -> None: "select exists(select 1 from review_policies where not legacy_incomplete " "or review_preference_window_seconds is not null " "or review_lease_duration_seconds is not null or configured_by_actor is not null " - "or configured_at is not null)" + "or configured_at is not null or legacy_requires_second_review is null)" ) ).scalar_one() unsafe_revision = bind.execute( sa.text( "select exists(select 1 from revision_policies where not legacy_incomplete " - "or configured_by_actor is not null or configured_at is not null)" + "or configured_by_actor is not null or configured_at is not null " + "or legacy_auto_reject_after_limit is null " + "or legacy_allowed_resubmission_states is null)" ) ).scalar_one() if unsafe_review or unsafe_revision: diff --git a/backend/app/modules/projects/service.py b/backend/app/modules/projects/service.py index 134e7da9..194bd17f 100644 --- a/backend/app/modules/projects/service.py +++ b/backend/app/modules/projects/service.py @@ -107,6 +107,7 @@ 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"} HASH_PATTERN = re.compile(r"^sha256:[0-9a-f]{64}$") HASH_TOKEN_PATTERN = re.compile(r"sha256:[0-9a-f]{64}") CONTENT_CID_PATTERN = re.compile( @@ -4296,6 +4297,12 @@ def _validate_activation_ready( or revision_policy.revision_deadline_hours < 1 ): raise GuideActivationBlocked("revision policy is incomplete") + if revision_policy.legacy_incomplete: + legacy_states = revision_policy._legacy_allowed_resubmission_states + if not legacy_states: + raise GuideActivationBlocked("revision policy is incomplete") + if not set(legacy_states).issubset(ALLOWED_REVISION_RESUBMISSION_STATES): + raise GuideActivationBlocked("revision policy contains invalid resubmission states") if payment_policy is None: raise GuideActivationBlocked("payment policy is required") if ( diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 9fe01979..eb26c735 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -22,7 +22,7 @@ DDL_LOCK_DIRECTORY = Path("/tmp") EXPECTED_PUBLIC_SCHEMA_SHA256 = ( - "61c9942bff66eab0c192303cb0e5e373d2586ed48fa7eb07df3e763189b48a88" + "d50e9eb30477e00a27b551b4c7714a020c605b67799c8d8e439287d0a6e512a9" ) PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 712408cc..c59309d6 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -2837,6 +2837,46 @@ async def row(table: str, row_id: str) -> dict: assert asyncio.run(row("revision_policies", revision_id)) == old_revision command.upgrade(config, "0034_review_revision_policy") + with pytest.raises(IntegrityError, match="legacy policy is immutable"): + asyncio.run( + execute( + "update review_policies set allowed_decisions='[]'::json where id=:id", + {"id": review_id}, + ) + ) + with pytest.raises(IntegrityError, match="legacy policy is immutable"): + asyncio.run( + execute( + "update revision_policies set max_revision_rounds=8 where id=:id", + {"id": revision_id}, + ) + ) + for table, trigger, column, restore in ( + ( + "review_policies", + "trg_review_policies_guard_write", + "legacy_requires_second_review", + "false", + ), + ( + "revision_policies", + "trg_revision_policies_guard_write", + "legacy_auto_reject_after_limit", + "true", + ), + ): + asyncio.run(execute(f"alter table {table} disable trigger {trigger}")) + asyncio.run(execute(f"update {table} set {column}=null")) + asyncio.run(execute(f"alter table {table} enable trigger {trigger}")) + with pytest.raises( + RuntimeError, + match="cannot downgrade canonical review or revision policy facts", + ): + command.downgrade(config, "0033_authorization_read_rate") + asyncio.run(execute(f"alter table {table} disable trigger {trigger}")) + asyncio.run(execute(f"update {table} set {column}={restore}")) + asyncio.run(execute(f"alter table {table} enable trigger {trigger}")) + asyncio.run( execute( "update review_policies set legacy_incomplete=false," @@ -2880,6 +2920,41 @@ async def row(table: str, row_id: str) -> dict: {"id": review_id}, ) ) + invalid_review_updates = ( + "allowed_decisions='[]'::json", + "max_active_review_leases_per_reviewer=2", + "self_review_allowed=true", + "reject_policy='reopen_to_pool'", + "finding_evidence_requirement='invalid'", + "configured_by_actor=''", + "legacy_incomplete=true", + ) + for mutation in invalid_review_updates: + with pytest.raises(IntegrityError): + asyncio.run( + execute( + f"update review_policies set {mutation} where id=:id", + {"id": review_id}, + ) + ) + asyncio.run( + execute( + "update revision_policies set legacy_incomplete=false," + "legacy_auto_reject_after_limit=null," + "legacy_allowed_resubmission_states=null," + "legacy_reviewer_reassignment_rule=null,configured_by_actor='actor-new' " + "where id=:id", + {"id": revision_id}, + ) + ) + for mutation in ("max_revision_rounds=0", "revision_deadline_hours=0"): + with pytest.raises(IntegrityError): + asyncio.run( + execute( + f"update revision_policies set {mutation} where id=:id", + {"id": revision_id}, + ) + ) with pytest.raises( RuntimeError, match="cannot downgrade canonical review or revision policy facts", @@ -2902,6 +2977,10 @@ async def row(table: str, row_id: str) -> dict: {"id": review_id}, ) ) + with pytest.raises(IntegrityError, match="cannot be deleted"): + asyncio.run( + execute("delete from review_policies where id=:id", {"id": review_id}) + ) with pytest.raises(IntegrityError, match="cannot be deleted"): asyncio.run( execute("delete from revision_policies where id=:id", {"id": revision_id}) @@ -2910,12 +2989,12 @@ async def row(table: str, row_id: str) -> dict: command.upgrade(config, "head") -def test_review_policy_write_serializes_with_publication( +def test_review_revision_policy_writes_serialize_with_publication( isolated_database_env: str, ) -> None: - """Prove policy-first and publication-first transactions serialize safely.""" + """Prove both policy tables serialize inserts/updates in both lock orders.""" - async def seed(marker: str) -> tuple[str, str]: + async def seed(marker: str, policy_kind: str, create_policy: bool) -> tuple[str, str]: project_id = str(uuid4()) policy_id = str(uuid4()) engine = create_async_engine(isolated_database_env) @@ -2936,99 +3015,97 @@ async def seed(marker: str) -> tuple[str, str]: ), {"id": str(uuid4()), "project_id": project_id}, ) - await connection.execute( - text( - "insert into review_policies " - "(id,project_id,guide_version,allowed_decisions,minimum_finding_fields," - "review_preference_window_seconds,review_lease_duration_seconds," - "max_active_review_leases_per_reviewer,self_review_allowed,reject_policy," - "finding_evidence_requirement,legacy_incomplete,configured_by_actor) " - "values (:id,:project_id,'v1'," - "'[\"accept\",\"needs_revision\",\"reject\"]'::json," - "'[\"description\",\"severity\"]'::json,900,1800,1,false," - "'close_task','optional',false,'actor')" - ), - {"id": policy_id, "project_id": project_id}, - ) + if create_policy: + await connection.execute( + text(policy_insert_sql(policy_kind)), + {"id": policy_id, "project_id": project_id}, + ) return project_id, policy_id finally: await engine.dispose() - async def policy_first() -> None: - project_id, policy_id = await seed(f"policy-first-{uuid4()}") + def policy_insert_sql(policy_kind: str) -> str: + if policy_kind == "review": + return ( + "insert into review_policies " + "(id,project_id,guide_version,allowed_decisions,minimum_finding_fields," + "review_preference_window_seconds,review_lease_duration_seconds," + "max_active_review_leases_per_reviewer,self_review_allowed,reject_policy," + "finding_evidence_requirement,legacy_incomplete,configured_by_actor) values " + "(:id,:project_id,'v1','[\"accept\",\"needs_revision\",\"reject\"]'::json," + "'[\"description\",\"severity\"]'::json,900,1800,1,false," + "'close_task','optional',false,'actor')" + ) + return ( + "insert into revision_policies " + "(id,project_id,guide_version,max_revision_rounds,revision_deadline_hours," + "legacy_incomplete,configured_by_actor) " + "values (:id,:project_id,'v1',7,48,false,'actor')" + ) + + def policy_write_sql(policy_kind: str, operation: str) -> str: + if operation == "insert": + return policy_insert_sql(policy_kind) + if policy_kind == "review": + return "update review_policies set review_lease_duration_seconds=1900 where id=:id" + return "update revision_policies set revision_deadline_hours=49 where id=:id" + + async def race(policy_kind: str, operation: str, policy_first: bool) -> None: + project_id, policy_id = await seed( + f"{policy_kind}-{operation}-{policy_first}-{uuid4()}", + policy_kind, + create_policy=operation == "update", + ) engine = create_async_engine(isolated_database_env) try: policy_connection = await engine.connect() publication_connection = await engine.connect() - policy_tx = await policy_connection.begin() - await policy_connection.execute( - text( - "update review_policies set review_lease_duration_seconds=1900 " - "where id=:id" - ), - {"id": policy_id}, - ) - - async def publish() -> None: - async with publication_connection.begin(): - await publication_connection.execute( - text( - "update project_guides set status='active'," - "effective_at=statement_timestamp() " - "where project_id=:project_id and version='v1'" - ), - {"project_id": project_id}, - ) - - publication = asyncio.create_task(publish()) - await asyncio.sleep(0.1) - assert not publication.done() - await policy_tx.commit() - await publication - await policy_connection.close() - await publication_connection.close() - finally: - await engine.dispose() + publication_sql = text( + "update project_guides set status='active',effective_at=statement_timestamp() " + "where project_id=:project_id and version='v1'" + ) + policy_sql = text(policy_write_sql(policy_kind, operation)) + parameters = {"id": policy_id, "project_id": project_id} + if policy_first: + first_tx = await policy_connection.begin() + await policy_connection.execute(policy_sql, parameters) + + async def publish() -> None: + async with publication_connection.begin(): + await publication_connection.execute( + publication_sql, {"project_id": project_id} + ) - async def publication_first() -> None: - project_id, policy_id = await seed(f"publication-first-{uuid4()}") - engine = create_async_engine(isolated_database_env) - try: - publication_connection = await engine.connect() - policy_connection = await engine.connect() - publication_tx = await publication_connection.begin() - await publication_connection.execute( - text( - "update project_guides set status='active'," - "effective_at=statement_timestamp() " - "where project_id=:project_id and version='v1'" - ), - {"project_id": project_id}, - ) + second = asyncio.create_task(publish()) + await asyncio.sleep(0.1) + assert not second.done() + await first_tx.commit() + await second + else: + first_tx = await publication_connection.begin() + await publication_connection.execute( + publication_sql, {"project_id": project_id} + ) - async def mutate() -> None: - async with policy_connection.begin(): - await policy_connection.execute( - text( - "update review_policies set review_lease_duration_seconds=1900 " - "where id=:id" - ), - {"id": policy_id}, - ) + async def write_policy() -> None: + async with policy_connection.begin(): + await policy_connection.execute(policy_sql, parameters) - mutation = asyncio.create_task(mutate()) - await asyncio.sleep(0.1) - assert not mutation.done() - await publication_tx.commit() - with pytest.raises(DBAPIError, match="published review and revision policies"): - await mutation + second = asyncio.create_task(write_policy()) + await asyncio.sleep(0.1) + assert not second.done() + await first_tx.commit() + with pytest.raises(DBAPIError, match="published review and revision policies"): + await second await publication_connection.close() await policy_connection.close() finally: await engine.dispose() - asyncio.run(policy_first()) - asyncio.run(publication_first()) + for policy_kind in ("review", "revision"): + for operation in ("insert", "update"): + for policy_first in (True, False): + asyncio.run(race(policy_kind, operation, policy_first)) def test_authorization_read_rate_scope_upgrade_and_downgrade_refusal( diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 2b38c7c3..5da951d7 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -49,6 +49,7 @@ from app.modules.projects.models import ( EffectiveProjectSubmissionArtifactPolicy, GuideSourceSnapshot, + PaymentPolicy, PostSubmitCheckerPolicy, PreSubmitCheckerPolicy, ProjectSetupRun, @@ -2710,6 +2711,46 @@ async def test_published_review_policy_rejects_same_version_mutation( assert after == before +async def test_work_context_uses_stamped_policy_values_after_payment_policy_mutation( + task_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + project = await create_active_project(task_client) + started_task = await create_started_task(task_client, project["id"], monkeypatch) + before_response = await task_client.get( + f"/api/v1/tasks/{started_task['id']}/work-context", + headers=auth_headers(), + ) + assert before_response.status_code == 200, before_response.text + before = before_response.json() + + async with db_session.get_session_factory()() as session: + payment_policy = await session.scalar( + select(PaymentPolicy).where( + PaymentPolicy.project_id == project["id"], + PaymentPolicy.guide_version == "v1", + ) + ) + assert payment_policy is not None + payment_policy.base_amount = Decimal("999.00") + payment_policy.currency = "EUR" + payment_policy.payout_type = "manual" + await session.commit() + + after_response = await task_client.get( + f"/api/v1/tasks/{started_task['id']}/work-context", + headers=auth_headers(), + ) + assert after_response.status_code == 200, after_response.text + after = after_response.json() + assert after["review_policy"] == before["review_policy"] + assert after["revision_policy"] == before["revision_policy"] + assert after["payment_policy"] == before["payment_policy"] + assert after["payment_policy"]["base_amount"] == "25.00" + assert after["payment_policy"]["currency"] == "USD" + assert after["payment_policy"]["payout_type"] == "fixed" + + async def test_task_context_apis_fail_closed_when_locked_context_is_missing( task_client: AsyncClient, ) -> None: From af094052edc6514cf256345e20c2906bcf8882e7 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:05:01 +0100 Subject: [PATCH 06/27] test(review): prove policy persistence boundaries --- backend/tests/test_alembic.py | 75 +++++++++++++++++++++++++++++-- backend/tests/test_projects.py | 82 +++++++++++++++++++++++++++++----- 2 files changed, 142 insertions(+), 15 deletions(-) diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index c59309d6..3ba2f645 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -2832,6 +2832,43 @@ async def row(table: str, row_id: str) -> dict: assert migrated_revision["legacy_incomplete"] is True assert migrated_revision["legacy_auto_reject_after_limit"] is True + forbidden_inserts = ( + ( + "insert into review_policies " + "(id,project_id,guide_version,allowed_decisions,minimum_finding_fields," + "legacy_incomplete) values " + "(:id,:project_id,'v1','[\"accept\",\"needs_revision\",\"reject\"]'::json," + "'[\"description\",\"severity\"]'::json,true)" + ), + ( + "insert into review_policies " + "(id,project_id,guide_version,allowed_decisions,minimum_finding_fields," + "legacy_incomplete) values " + "(:id,:project_id,'v1','[\"accept\",\"needs_revision\",\"reject\"]'::json," + "'[\"description\",\"severity\"]'::json,false)" + ), + ( + "insert into revision_policies " + "(id,project_id,guide_version,max_revision_rounds," + "revision_deadline_hours,legacy_incomplete) " + "values (:id,:project_id,'v1',7,48,true)" + ), + ( + "insert into revision_policies " + "(id,project_id,guide_version,max_revision_rounds," + "revision_deadline_hours,legacy_incomplete) " + "values (:id,:project_id,'v1',7,48,false)" + ), + ) + for statement in forbidden_inserts: + with pytest.raises(IntegrityError): + asyncio.run( + execute( + statement, + {"id": str(uuid4()), "project_id": project_id}, + ) + ) + command.downgrade(config, "0033_authorization_read_rate") assert asyncio.run(row("review_policies", review_id)) == old_review assert asyncio.run(row("revision_policies", revision_id)) == old_revision @@ -3057,9 +3094,34 @@ async def race(policy_kind: str, operation: str, policy_first: bool) -> None: create_policy=operation == "update", ) engine = create_async_engine(isolated_database_env) + policy_connection = None + publication_connection = None + monitor_connection = None try: policy_connection = await engine.connect() publication_connection = await engine.connect() + monitor_connection = await engine.connect() + policy_pid = await policy_connection.scalar(text("select pg_backend_pid()")) + publication_pid = await publication_connection.scalar( + text("select pg_backend_pid()") + ) + await policy_connection.commit() + await publication_connection.commit() + + async def require_lock_wait(pid: int) -> None: + for _ in range(100): + waiting = await monitor_connection.scalar( + text( + "select exists(select 1 from pg_stat_activity " + "where pid=:pid and state='active' and wait_event_type='Lock')" + ), + {"pid": pid}, + ) + if waiting: + return + await asyncio.sleep(0.01) + pytest.fail(f"backend {pid} never entered a database lock wait") + publication_sql = text( "update project_guides set status='active',effective_at=statement_timestamp() " "where project_id=:project_id and version='v1'" @@ -3077,7 +3139,7 @@ async def publish() -> None: ) second = asyncio.create_task(publish()) - await asyncio.sleep(0.1) + await require_lock_wait(publication_pid) assert not second.done() await first_tx.commit() await second @@ -3092,14 +3154,19 @@ async def write_policy() -> None: await policy_connection.execute(policy_sql, parameters) second = asyncio.create_task(write_policy()) - await asyncio.sleep(0.1) + await require_lock_wait(policy_pid) assert not second.done() await first_tx.commit() with pytest.raises(DBAPIError, match="published review and revision policies"): await second - await publication_connection.close() - await policy_connection.close() finally: + for connection in ( + monitor_connection, + publication_connection, + policy_connection, + ): + if connection is not None: + await connection.close() await engine.dispose() for policy_kind in ("review", "revision"): diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index c03b1c09..4354decd 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -13,7 +13,7 @@ import pytest # type: ignore[import-not-found] from httpx import ASGITransport, AsyncClient -from sqlalchemy import event, select, update +from sqlalchemy import event, select, text, update from sqlalchemy.dialects import postgresql from sqlalchemy.exc import IntegrityError from sqlalchemy.schema import CreateIndex @@ -7205,12 +7205,25 @@ async def test_activation_requires_complete_payment_policy(project_client: Async assert "payment policy is incomplete" in response.json()["detail"] -async def test_revision_policy_rejects_retired_resubmission_state_input( +@pytest.mark.parametrize( + ("policy_name", "field_name", "field_value"), + ( + ("review_policy", "requires_second_review", False), + ("review_policy", "sla_hours", 24), + ("revision_policy", "auto_reject_after_limit", True), + ("revision_policy", "allowed_resubmission_states", ["needs_revision"]), + ("revision_policy", "reviewer_reassignment_rule", "same reviewer preferred"), + ), +) +async def test_policy_schema_rejects_retired_archival_inputs( project_client: AsyncClient, + policy_name: str, + field_name: str, + field_value: object, ) -> None: project = await create_project(project_client) payload = complete_guide_payload() - payload["revision_policy"]["allowed_resubmission_states"] = [] + payload[policy_name][field_name] = field_value response = await project_client.post( f"/api/v1/projects/{project['id']}/guides", headers=auth_headers(), @@ -7218,7 +7231,7 @@ async def test_revision_policy_rejects_retired_resubmission_state_input( ) assert response.status_code == 422 - assert "allowed_resubmission_states" in response.text + assert field_name in response.text async def test_revision_policy_requires_deadline(project_client: AsyncClient) -> None: @@ -7258,21 +7271,65 @@ async def test_guide_update_rejects_manual_post_submit_checker_policy( assert "post_submit_checker_policy" in response.text -async def test_revision_policy_rejects_retired_reassignment_input( +@pytest.mark.parametrize( + ("legacy_states", "expected_status", "expected_detail"), + ( + (None, 422, "revision policy is incomplete"), + ([], 422, "revision policy is incomplete"), + (["submitted"], 422, "revision policy contains invalid resubmission states"), + (["needs_revision"], 200, None), + ), +) +async def test_activation_preserves_migrated_revision_policy_state_rules( project_client: AsyncClient, + legacy_states: list[str] | None, + expected_status: int, + expected_detail: str | None, ) -> None: project = await create_project(project_client) - payload = complete_guide_payload() - payload["revision_policy"]["reviewer_reassignment_rule"] = "same reviewer preferred" + guide = await create_guide(project_client, project["id"], complete_guide_payload()) + await create_approved_policy_bundle(project_client, project["id"], guide["id"]) + + async with db_session.get_session_factory()() as session: + await session.execute( + text("alter table revision_policies disable trigger trg_revision_policies_guard_write") + ) + await session.execute( + text( + "update revision_policies set legacy_incomplete=true," + "configured_by_actor=null,configured_at=null," + "legacy_auto_reject_after_limit=true," + "legacy_allowed_resubmission_states=cast(:states as json)," + "legacy_reviewer_reassignment_rule='same reviewer preferred' " + "where project_id=:project_id and guide_version='v1'" + ), + { + "project_id": project["id"], + "states": json.dumps(legacy_states) if legacy_states is not None else None, + }, + ) + await session.execute( + text("alter table revision_policies enable trigger trg_revision_policies_guard_write") + ) + await session.commit() response = await project_client.post( - f"/api/v1/projects/{project['id']}/guides", + f"/api/v1/projects/{project['id']}/guides/{guide['id']}/activate", headers=auth_headers(), - json=payload, ) - assert response.status_code == 422 - assert "reviewer_reassignment_rule" in response.text + assert response.status_code == expected_status, response.text + if expected_detail is not None: + assert expected_detail in response.json()["detail"] + else: + revision_policy = response.json()["revision_policy"] + assert revision_policy["legacy_incomplete"] is True + for archival_field in ( + "auto_reject_after_limit", + "allowed_resubmission_states", + "reviewer_reassignment_rule", + ): + assert archival_field not in revision_policy async def test_activation_rejects_pending_pre_submit_checker_policy( @@ -7484,10 +7541,13 @@ async def test_guide_activation_and_active_guide_retrieval(project_client: Async assert active.json()["review_policy"]["review_lease_duration_seconds"] == 1800 assert active.json()["review_policy"]["finding_evidence_requirement"] == "optional" assert active.json()["review_policy"]["legacy_incomplete"] is False + assert "requires_second_review" not in active.json()["review_policy"] assert "sla_hours" not in active.json()["review_policy"] assert active.json()["revision_policy"]["max_revision_rounds"] == 7 assert active.json()["revision_policy"]["legacy_incomplete"] is False assert "auto_reject_after_limit" not in active.json()["revision_policy"] + assert "allowed_resubmission_states" not in active.json()["revision_policy"] + assert "reviewer_reassignment_rule" not in active.json()["revision_policy"] assert active.json()["payment_policy"]["base_amount"] == "25.00" From 35531df254c6b25726d666a5e89eda997b97d792 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:15:33 +0100 Subject: [PATCH 07/27] test(review): close policy evidence gaps --- backend/tests/test_alembic.py | 33 +++++++++++++++++++++++++-------- backend/tests/test_projects.py | 20 ++++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 3ba2f645..ae1d8173 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -2832,36 +2832,53 @@ async def row(table: str, row_id: str) -> dict: assert migrated_revision["legacy_incomplete"] is True assert migrated_revision["legacy_auto_reject_after_limit"] is True + for version in ("v2", "v3", "v4", "v5"): + asyncio.run( + execute( + "insert into project_guides " + "(id,project_id,version,status,content_markdown,created_by) " + "values (:id,:project_id,:version,'draft','# Guide','actor-old')", + { + "id": str(uuid4()), + "project_id": project_id, + "version": version, + }, + ) + ) forbidden_inserts = ( ( "insert into review_policies " "(id,project_id,guide_version,allowed_decisions,minimum_finding_fields," "legacy_incomplete) values " - "(:id,:project_id,'v1','[\"accept\",\"needs_revision\",\"reject\"]'::json," - "'[\"description\",\"severity\"]'::json,true)" + "(:id,:project_id,'v2','[\"accept\",\"needs_revision\",\"reject\"]'::json," + "'[\"description\",\"severity\"]'::json,true)", + "new policies cannot claim legacy state", ), ( "insert into review_policies " "(id,project_id,guide_version,allowed_decisions,minimum_finding_fields," "legacy_incomplete) values " - "(:id,:project_id,'v1','[\"accept\",\"needs_revision\",\"reject\"]'::json," - "'[\"description\",\"severity\"]'::json,false)" + "(:id,:project_id,'v3','[\"accept\",\"needs_revision\",\"reject\"]'::json," + "'[\"description\",\"severity\"]'::json,false)", + "ck_review_policies_complete_or_legacy", ), ( "insert into revision_policies " "(id,project_id,guide_version,max_revision_rounds," "revision_deadline_hours,legacy_incomplete) " - "values (:id,:project_id,'v1',7,48,true)" + "values (:id,:project_id,'v4',7,48,true)", + "new policies cannot claim legacy state", ), ( "insert into revision_policies " "(id,project_id,guide_version,max_revision_rounds," "revision_deadline_hours,legacy_incomplete) " - "values (:id,:project_id,'v1',7,48,false)" + "values (:id,:project_id,'v5',7,48,false)", + "ck_revision_policies_complete_or_legacy", ), ) - for statement in forbidden_inserts: - with pytest.raises(IntegrityError): + for statement, expected_error in forbidden_inserts: + with pytest.raises(IntegrityError, match=expected_error): asyncio.run( execute( statement, diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index 4354decd..825a04b9 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -7291,9 +7291,22 @@ async def test_activation_preserves_migrated_revision_policy_state_rules( await create_approved_policy_bundle(project_client, project["id"], guide["id"]) async with db_session.get_session_factory()() as session: + await session.execute( + text("alter table review_policies disable trigger trg_review_policies_guard_write") + ) await session.execute( text("alter table revision_policies disable trigger trg_revision_policies_guard_write") ) + await session.execute( + text( + "update review_policies set legacy_incomplete=true," + "review_preference_window_seconds=null,review_lease_duration_seconds=null," + "configured_by_actor=null,configured_at=null," + "legacy_requires_second_review=false,legacy_sla_hours=24 " + "where project_id=:project_id and guide_version='v1'" + ), + {"project_id": project["id"]}, + ) await session.execute( text( "update revision_policies set legacy_incomplete=true," @@ -7308,6 +7321,9 @@ async def test_activation_preserves_migrated_revision_policy_state_rules( "states": json.dumps(legacy_states) if legacy_states is not None else None, }, ) + await session.execute( + text("alter table review_policies enable trigger trg_review_policies_guard_write") + ) await session.execute( text("alter table revision_policies enable trigger trg_revision_policies_guard_write") ) @@ -7322,6 +7338,10 @@ async def test_activation_preserves_migrated_revision_policy_state_rules( if expected_detail is not None: assert expected_detail in response.json()["detail"] else: + review_policy = response.json()["review_policy"] + assert review_policy["legacy_incomplete"] is True + for archival_field in ("requires_second_review", "sla_hours"): + assert archival_field not in review_policy revision_policy = response.json()["revision_policy"] assert revision_policy["legacy_incomplete"] is True for archival_field in ( From 5417578dadec3da7e424d80f4371173aec0d50b8 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:19:43 +0100 Subject: [PATCH 08/27] docs(review): publish 03P trust evidence --- .../REVIEW_LOG.md | 17 +++ .../STATUS.md | 25 +++-- ...-03P-review-revision-policy-persistence.md | 6 +- ...WS-REV-001-03P-internal-review-evidence.md | 51 +++++++++ .../reviews/WS-REV-001-03P-pr-trust-bundle.md | 100 ++++++++++++++++++ .agent-loop/merge-intents/WS-REV-001-03P.json | 9 ++ 6 files changed, 193 insertions(+), 15 deletions(-) create mode 100644 .agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md create mode 100644 .agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md create mode 100644 .agent-loop/merge-intents/WS-REV-001-03P.json diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md index 877ce163..f787bae2 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md @@ -1123,3 +1123,20 @@ AUTH/ART/CON owner plan, or frozen reference source changed. The repaired candidate requires fresh exact-SHA internal review, deterministic gates, evidence rebinding, push, CodeRabbit re-review, and GitHub CI before PR #150 is ready for the user's merge decision. + +## WS-REV-001-03P Internal Implementation Review - 2026-07-23 + +Signed run `30014647556` authorized 03P on exact main `bcf1292e`. The bounded +implementation persists immutable ReviewPolicy and RevisionPolicy facts only. +It preserves migrated legacy facts privately, permits only an atomic legacy to +canonical conversion, serializes policy writes with guide publication, refuses +lossy downgrade, and exposes no retired archival fields. + +Review iterations repaired legacy activation compatibility, stamped Task +compensation isolation, deterministic PostgreSQL waiter proof, all-five-field +archival denial/non-exposure, and distinct-context direct-insert failures. +Exact code candidate `35531df254c6b25726d666a5e89eda997b97d792` +passes senior engineering, QA/test, security/auth, product/ops, architecture, +docs, reuse/dedup, test-delta, CI-integrity, and circuit-breaker review. No +finding remains. Full backend tests and coverage are intentionally deferred to +GitHub Actions; 03A has not started. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md index 6d18ec34..40f921fd 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md @@ -2,19 +2,18 @@ ## Current status -On 2026-07-22 a complete reread of the canonical Markdown and PDF specification -identified that the 02A family crossed the REV ownership boundary. REV begins -only when the current finalized checker result is `allow_review`; it consumes -the existing Submission and the same submitted/verified artifact set. Project -Guide setup, activation, Task intake, and upstream publication fencing belong to -their owning subsystems. REV reports gaps in those handoffs and does not repair -them. The attempted 02A1 runtime candidate was reverted in full. - -`WS-REV-001-PLAN3` is proposed planning-only work, not canonically active. It retires -02A/02A1/02A2/02A3/02A4/02B/02C as REV implementation authorization. The first -future REV chunk is 03P: REV-owned ReviewPolicy/RevisionPolicy persistence. -Canonical start requires the signed `Loop Memory Explicit Event` workflow on -exact current main; chat and local work are not start evidence. +`WS-REV-001-03P` is active through signed `Loop Memory Explicit Event` run +`30014647556` on exact trusted main +`bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Implementation candidate +`35531df254c6b25726d666a5e89eda997b97d792` persists only REV-owned immutable +ReviewPolicy and RevisionPolicy facts and has passed every required internal +review track plus focused local evidence. GitHub Actions full-suite/coverage, +CodeRabbit, and human review remain before merge. + +The chunk does not implement Project Guide ownership, Task transitions, +Submission or artifact behavior, review queues or leases, Review decisions, +revision execution, FinalAcceptance, or contribution records. `03A` remains +stopped and requires a separate signed start after 03P merges. The historical text below records the superseded PLAN2/02A state and is not current implementation authority. Trusted main at that time was `44f2467c`, which additionally 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 626fb24a..86538519 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 @@ -3,8 +3,10 @@ ## Status Active through signed `Loop Memory Explicit Event` run `30014647556` on exact -trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Implementation remains -gated on pre-implementation review of this refreshed contract. +trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Implementation candidate +`35531df254c6b25726d666a5e89eda997b97d792` passed every required internal +review track and focused local gate. Publication remains gated on final evidence +review and current-head GitHub Actions, CodeRabbit, and human review. ## Goal diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md new file mode 100644 index 00000000..9b91c46e --- /dev/null +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -0,0 +1,51 @@ +# Internal Review Evidence: WS-REV-001-03P + +## Candidate + +- Trusted base: `bcf1292e1a591e3e84bf8ee212ee7191d80741fa` +- Reviewed code candidate: `35531df254c6b25726d666a5e89eda997b97d792` +- Signed start: workflow run `30014647556` +- Risk: L1 / P2 +- Boundary: ReviewPolicy and RevisionPolicy persistence only + +## Reviewer results + +| Tracks | Result | Findings closed | +|---|---:|---| +| Senior engineering, architecture, reuse/dedup, circuit breaker | PASS | Replaced timing inference with exact-backend PostgreSQL lock-wait proof. | +| QA/test, product/ops, test delta | PASS | Restored legacy activation and Task snapshot proof; covered every archival field and insert boundary. | +| Security/auth, docs, CI integrity | PASS | Proved fail-closed conversion, publication races, downgrade refusal, provenance, and no gate weakening. | + +No Critical, High, Medium, or Low finding remains. All reviewer sessions +completed before publication. + +## Deterministic evidence + +- Alembic sole head: `0034_review_revision_policy`. +- Migration/direct-SQL/lossless downgrade test: PASS, including both policy + tables, exact archival reconstruction, insert/update/delete refusal, and + legacy-to-canonical conversion. +- Publication race test: PASS for review/revision, insert/update, and both lock + orders; every second backend was observed waiting on a PostgreSQL lock before + the first transaction released it. +- Project policy selector: PASS, 13 tests; migrated missing, empty, invalid, and + valid resubmission-state outcomes are preserved and all five archives remain + private. +- Task policy selector: PASS, 2 tests; immutable review/revision snapshots and + all compensation assertions remain intact. +- Artifact compatibility node: PASS, 1 test; no ART assertion changed. +- Ruff: PASS for every changed backend/test/migration path. +- Stale wording and Markdown links: PASS. +- Agent gates: PASS, 100 tests. +- `git diff --check`: PASS. + +The full backend suite and repository/subsystem coverage gates were not run +locally. They must run in GitHub Actions per the user's scaling instruction. + +## Scope and disposition + +No Task lifecycle, Project Guide behavior, Submission, checker, AUTH, ART, CON, +queue, lease, Review decision, revision execution, FinalAcceptance, or +contribution implementation was added. The reviewed atomic L1 size exception +is documented in the chunk contract. PASS for publication after the +evidence-only delta is reviewed; 03A remains stopped. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md new file mode 100644 index 00000000..7a1c2026 --- /dev/null +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md @@ -0,0 +1,100 @@ +# PR Trust Bundle: WS-REV-001-03P + +## Chunk + +`WS-REV-001-03P` - Review And Revision Policy Persistence. + +## Goal + +Persist the immutable REV-owned policy facts required by later review routing, +leases, decisions, and human revision work, without implementing those later +lifecycle stages. + +## Human-approved intent + +REV starts only after `allow_review`, consumes the existing Submission and +artifact set, records complete traceable review/revision history in later +chunks, and never absorbs Project Guide, Task, AUTH, ART, or CON ownership. + +## What changed and why + +Migration 0034 replaces ambiguous legacy policy columns with explicit review +preference/lease, finding-evidence, revision-limit/deadline, and provenance +facts. Legacy values remain private and losslessly downgradeable until one +atomic conversion. Models, schemas, repository/service mapping, active docs, +and focused tests now agree with that database contract. + +## Design chosen + +Separate table-specific triggers make policy rows immutable, serialize every +insert/update with the exact guide row, reject runtime legacy claims, and permit +only atomic legacy-to-canonical conversion. Downgrade locks all three involved +tables and refuses canonical facts or unreconstructible archives before DDL. + +## Alternatives rejected + +- No mutable published policy or delete path. +- No runtime fallback defaults for migrated incomplete rows. +- No combined queue, lease, Review, decision, revision, or contribution work. +- No AUTH-owned actor schema or ART/Submission behavior. +- No lossy downgrade. + +## Scope control and product behavior + +The change is limited to the contract's policy persistence, mapping, docs, and +proof-only compatibility exceptions. Canonical decisions remain exactly +`accept`, `needs_revision`, and `reject`. No review lifecycle transition is +activated by this PR. + +## Acceptance criteria proof + +Direct SQL proves canonical shape, provenance, private archives, immutable +identity/context, update/delete refusal, runtime-insert denial, atomic +conversion, downgrade refusal, and lossless legacy round-trip. Eight real +PostgreSQL races prove both policy tables and both write operations serialize +with guide publication in both lock orders. + +## Tests/checks run + +Focused migration, Project, Task, and artifact tests passed. Alembic has one +head, Ruff passed all changed backend paths, stale wording and Markdown links +passed, 100 agent-gate tests passed, and diff integrity passed. Full backend +tests and coverage are delegated to GitHub Actions as instructed by the user. + +## Test delta and CI integrity + +Tests add constraints, downgrade, deterministic waiter, legacy compatibility, +archive privacy, and immutable snapshot proof. Existing compensation and ART +assertions remain intact. No workflow, package script, skip, xfail, coverage +floor, or assertion was weakened; CI must preserve 78 percent repository-wide +coverage and prove at least 90 percent for the materially changed subsystem. + +## Reviewer results + +Senior engineering, QA/test, security/auth, product/ops, architecture, docs, +reuse/dedup, test delta, CI integrity, and circuit breaker all PASS on code +candidate `35531df254c6b25726d666a5e89eda997b97d792` after three repair rounds. + +## External review + +Pending publication. GitHub Actions and CodeRabbit must pass on the final pushed +head, and every actionable external comment must be resolved before merge. + +## Remaining risks and follow-up work + +Later queue, lease, Review chain, decision, revision, FinalAcceptance, and CON +composition remain deliberately unimplemented. After merge, loop memory may +name same-initiative successor 03A, but 03A requires a separate explicit signed +start and a fresh current-main dependency check. + +## Human review focus + +Review the trigger invariants, downgrade refusal/reconstruction, eight race +cases, legacy API privacy, strict REV boundary, and preservation of Task and +ART compatibility assertions. + +## Human merge ownership + +Only the user may approve and merge this specific PR. Do not merge with a +pending/failed current-head GitHub or CodeRabbit check or an unresolved +actionable comment. Merge does not start 03A. diff --git a/.agent-loop/merge-intents/WS-REV-001-03P.json b/.agent-loop/merge-intents/WS-REV-001-03P.json new file mode 100644 index 00000000..e8cc629c --- /dev/null +++ b/.agent-loop/merge-intents/WS-REV-001-03P.json @@ -0,0 +1,9 @@ +{ + "chunk_id": "WS-REV-001-03P", + "chunk_title": "Review And Revision Policy Persistence", + "initiative_id": "WS-REV-001", + "next_chunk_id": "WS-REV-001-03A", + "next_chunk_title": "Queue And Lease Base Persistence", + "next_requires_explicit_start": true, + "schema_version": 2 +} From 2a5687067dae754d5df6cfbbb0e8ccaf5b851e5a Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:23:35 +0100 Subject: [PATCH 09/27] docs(review): bind machine-readable evidence --- .../STATUS.md | 4 +-- ...WS-REV-001-03P-internal-review-evidence.md | 28 +++++++++++++++---- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md index 40f921fd..13e45729 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md @@ -25,11 +25,11 @@ database/concurrency boundaries. That attempted split is now retired. ## Historical dependency snapshot -Current trusted main is `14fa4316f7d984f2176657bfafd2a2dae56f944e` +At the PLAN3 snapshot, trusted main was `14fa4316f7d984f2176657bfafd2a2dae56f944e` with sole Alembic head `0033_authorization_read_rate_control`. The bullets below describe the older PLAN2 snapshot and are not current runtime proof. -Canonical signed loop memory currently remains stopped after merged 02A and +At that snapshot, canonical signed loop memory remained stopped after merged 02A and names retired 02A1 as next. That projection is accurate historical automation state but no longer a valid REV scope choice. PLAN3 is proposed—not active—and its reviewed merge intent changes the same-initiative successor to 03P. No diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 9b91c46e..40c068b3 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -8,13 +8,31 @@ - Risk: L1 / P2 - Boundary: ReviewPolicy and RevisionPolicy persistence only +Open sub-agent sessions: none + +Valid findings addressed: yes + +## Reviewed Revision + +Reviewed code SHA: `5417578dadec3da7e424d80f4371173aec0d50b8` + +Reviewed at: 2026-07-23T16:21:15Z + +Reviewer run IDs: /root/plan_arch_review@5417578dadec3da7e424d80f4371173aec0d50b8; /root/qa_product_review@5417578dadec3da7e424d80f4371173aec0d50b8; /root/security_docs_ci_review@5417578dadec3da7e424d80f4371173aec0d50b8 + ## Reviewer results -| Tracks | Result | Findings closed | -|---|---:|---| -| Senior engineering, architecture, reuse/dedup, circuit breaker | PASS | Replaced timing inference with exact-backend PostgreSQL lock-wait proof. | -| QA/test, product/ops, test delta | PASS | Restored legacy activation and Task snapshot proof; covered every archival field and insert boundary. | -| Security/auth, docs, CI integrity | PASS | Proved fail-closed conversion, publication races, downgrade refusal, provenance, and no gate weakening. | +| Reviewer | Result | Blocking findings | Notes | +|---|---:|---|---| +| Senior engineering | PASS | None | Maintainable table-specific guards and bounded atomic L1 scope. | +| QA/test | PASS | None | Focused migration, API, snapshot, and compatibility proof is complete. | +| Security/auth | PASS | None | Provenance and immutability fail closed without absorbing AUTH ownership. | +| Product/ops | PASS | None | Policy persistence remains separate from later lifecycle execution. | +| Architecture | PASS | None | No Project Guide, Task, Submission, ART, AUTH, or CON boundary drift. | +| CI integrity | PASS | None | No workflow, coverage floor, script, skip, or assertion was weakened. | +| Docs | PASS | None | Active docs, schema, status, and lifecycle terminology agree. | +| Reuse/dedup | PASS | None | Existing Project policy mapping and locking conventions are reused. | +| Test delta | PASS | None | Compensation and ART assertions remain intact; proof only increased. | No Critical, High, Medium, or Low finding remains. All reviewer sessions completed before publication. From 108a21625c46f62f899bbcbd186353529205ba5a Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:29:21 +0100 Subject: [PATCH 10/27] docs(review): authorize API policy fixture repair --- .../WS-REV-001-03P-review-revision-policy-persistence.md | 5 +++++ 1 file changed, 5 insertions(+) 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 86538519..045b2e56 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 @@ -30,6 +30,7 @@ backend/tests/test_projects.py backend/tests/test_tasks.py backend/tests/test_artifact_admission.py backend/tests/conftest.py +backend/scripts/api_contract_e2e.py docs/architecture_data_model.md docs/template_project_guide.md .agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/{DISCOVERY,PLAN,STATUS,REVIEW_LOG}.md @@ -54,6 +55,10 @@ The three cross-owner test files are proof-only exceptions: while preserving every existing Project Guide activation outcome. Retired request fields may become schema-denial tests, but activation denial may not be removed or broadened by 03P. +- `backend/scripts/api_contract_e2e.py`: update only the existing Project Guide + policy request fixture for the canonical 03P schema. No API lifecycle step, + endpoint expectation, authorization assertion, or non-policy fixture may + change. ## Not allowed From 0c474f1edc3b683b774e998f0ad32190bab339e0 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:30:45 +0100 Subject: [PATCH 11/27] docs(review): clarify proof-only exceptions --- .../chunks/WS-REV-001-03P-review-revision-policy-persistence.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 045b2e56..251d9f78 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 @@ -39,7 +39,7 @@ docs/template_project_guide.md .agent-loop/merge-intents/WS-REV-001-03P.json ``` -The three cross-owner test files are proof-only exceptions: +The following cross-owner files are proof-only exceptions: - `backend/tests/conftest.py`: update only the deterministic public-schema fingerprint for migration 0034. From b68a1e22b3bf373d15479784d545fcc9b5737f64 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:31:39 +0100 Subject: [PATCH 12/27] fix(review): align API contract policy fixture --- backend/scripts/api_contract_e2e.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/backend/scripts/api_contract_e2e.py b/backend/scripts/api_contract_e2e.py index 60dfc298..5dba947b 100644 --- a/backend/scripts/api_contract_e2e.py +++ b/backend/scripts/api_contract_e2e.py @@ -641,17 +641,14 @@ def guide_payload(run_id: str) -> dict: ), "change_summary": "Initial real API guide", "review_policy": { - "requires_second_review": False, "allowed_decisions": ["accept", "needs_revision", "reject"], - "minimum_finding_fields": ["issue", "required_fix"], - "sla_hours": 24, + "minimum_finding_fields": ["description", "severity"], + "review_preference_window_seconds": 900, + "review_lease_duration_seconds": 1800, }, "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", }, "payment_policy": { "base_amount": "25.00", From 94722ffd4dedb7e6352458189735c74987b1c3c9 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:33:21 +0100 Subject: [PATCH 13/27] docs(review): record API E2E repair --- .../REVIEW_LOG.md | 11 ++++++ ...WS-REV-001-03P-external-review-response.md | 35 +++++++++++++++++++ ...WS-REV-001-03P-internal-review-evidence.md | 6 ++-- .../reviews/WS-REV-001-03P-pr-trust-bundle.md | 8 +++-- 4 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 .agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md index f787bae2..17c4e127 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md @@ -1140,3 +1140,14 @@ passes senior engineering, QA/test, security/auth, product/ops, architecture, docs, reuse/dedup, test-delta, CI-integrity, and circuit-breaker review. No finding remains. Full backend tests and coverage are intentionally deferred to GitHub Actions; 03A has not started. + +## WS-REV-001-03P API E2E Repair - 2026-07-23 + +PR #195's first GitHub run passed preflight and Agent Gates but API E2E rejected +the stale Project Guide policy fixture. A prospective contract amendment +authorized only `backend/scripts/api_contract_e2e.py::guide_payload`; all +lifecycle, endpoint, authorization, response, and non-policy behavior remained +prohibited. Candidate `b68a1e22b3bf373d15479784d545fcc9b5737f64` +removes the five retired inputs, uses canonical finding fields, and supplies the +two reviewed durations. Ruff and diff integrity pass, and all required internal +tracks pass on the exact candidate. Fresh GitHub proof remains required. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md new file mode 100644 index 00000000..2a789b11 --- /dev/null +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md @@ -0,0 +1,35 @@ +# External Review Response: WS-REV-001-03P + +## Comments addressed + +- GitHub Backend run `30025075528`, job `89267629866`, failed API E2E because + `guide_payload()` still sent five retired policy fields, used obsolete finding + names, and omitted the two canonical review durations. +- A prospectively reviewed scope amendment authorized only the policy request + fixture. Candidate `b68a1e22b3bf373d15479784d545fcc9b5737f64` + removes those retired inputs, uses `description` and `severity`, and supplies + 900-second preference and 1800-second lease durations. + +## Comments deferred + +None. + +## Human decisions needed + +Only the normal explicit approval to merge PR #195 after every current-head +external check passes. The repair does not start 03A. + +## Commands rerun + +- Ruff on `backend/scripts/api_contract_e2e.py`: PASS. +- `git diff --check`: PASS. +- Senior/architecture/reuse, QA/product/test-delta, and security/docs/CI + exact-SHA review: PASS. +- Fresh API E2E and full-suite/coverage evidence: pending GitHub Actions after + push. + +## Remaining risks + +No CodeRabbit finding is open. The first CodeRabbit status passed while noting +service rate limiting, so the final pushed head still requires a fresh status. +No lifecycle or authorization assertion was changed to obtain the repair. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 40c068b3..8028d4ce 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -14,11 +14,11 @@ Valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `5417578dadec3da7e424d80f4371173aec0d50b8` +Reviewed code SHA: `b68a1e22b3bf373d15479784d545fcc9b5737f64` -Reviewed at: 2026-07-23T16:21:15Z +Reviewed at: 2026-07-23T16:32:45Z -Reviewer run IDs: /root/plan_arch_review@5417578dadec3da7e424d80f4371173aec0d50b8; /root/qa_product_review@5417578dadec3da7e424d80f4371173aec0d50b8; /root/security_docs_ci_review@5417578dadec3da7e424d80f4371173aec0d50b8 +Reviewer run IDs: /root/plan_arch_review@b68a1e22b3bf373d15479784d545fcc9b5737f64; /root/qa_product_review@b68a1e22b3bf373d15479784d545fcc9b5737f64; /root/security_docs_ci_review@b68a1e22b3bf373d15479784d545fcc9b5737f64 ## Reviewer results diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md index 7a1c2026..5150c8d7 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md @@ -77,8 +77,12 @@ candidate `35531df254c6b25726d666a5e89eda997b97d792` after three repair rounds. ## External review -Pending publication. GitHub Actions and CodeRabbit must pass on the final pushed -head, and every actionable external comment must be resolved before merge. +PR #195's first run passed preflight and Agent Gates. CodeRabbit returned pass +with no actionable comment while reporting service rate limiting. API E2E found +that its existing Project Guide fixture still sent retired policy fields. The +prospectively reviewed repair changes only that fixture to the canonical 03P +schema; exact-SHA internal review and Ruff pass. Fresh current-head GitHub +Actions and CodeRabbit checks remain required before merge. ## Remaining risks and follow-up work From 5a29a1552bf84496d97a0e285868350256ab5c7f Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:43:48 +0100 Subject: [PATCH 14/27] test(review): repair full-suite policy compatibility --- backend/tests/test_alembic.py | 12 ++++++------ backend/tests/test_artifact_admission.py | 23 ++++++++++++----------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index ae1d8173..3da2f588 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -135,7 +135,7 @@ def test_artifact_recovery_schema_and_empty_downgrade( command.downgrade(config, "base") command.upgrade(config, "head") assert asyncio.run(_artifact_recovery_schema(isolated_database_env)) == { - "revision": "0033_authorization_read_rate", + "revision": "0034_review_revision_policy", "constraints": { "artifact_recovery_attempt_custody", "artifact_verification_lineage_custody", @@ -201,7 +201,7 @@ def test_project_role_migration_constraints_and_immutable_history( command.upgrade(config, "head") result = asyncio.run(_exercise_project_role_migration(isolated_database_env)) assert result == { - "revision": "0033_authorization_read_rate", + "revision": "0034_review_revision_policy", "role_count": 3, "invalid_availability": "23514", "duplicate_role": "23505", @@ -409,7 +409,7 @@ def test_project_role_downgrade_refuses_each_reserved_evidence_predicate( ): command.downgrade(config, "0030_artifact_verification") assert asyncio.run(_project_role_refusal_state(isolated_database_env))[:3] == ( - "0033_authorization_read_rate", + "0034_review_revision_policy", True, True, ) @@ -436,7 +436,7 @@ def test_project_role_downgrade_refuses_each_reserved_evidence_predicate( ): command.downgrade(config, "0030_artifact_verification") assert asyncio.run(_project_role_refusal_state(isolated_database_env))[:3] == ( - "0033_authorization_read_rate", + "0034_review_revision_policy", True, True, ) @@ -464,7 +464,7 @@ def test_outbox_migration_schema_and_downgrade_writer_guard( command.upgrade(config, "head") schema = asyncio.run(_outbox_schema(isolated_database_env)) assert schema == { - "revision": "0033_authorization_read_rate", + "revision": "0034_review_revision_policy", "columns": { "aggregate_id", "aggregate_type", @@ -524,7 +524,7 @@ def test_outbox_migration_schema_and_downgrade_writer_guard( ) assert committed == "refused_after_commit" assert asyncio.run(_current_revision(isolated_database_env)) == ( - "0033_authorization_read_rate" + "0034_review_revision_policy" ) asyncio.run(_remove_outbox_migration_row(isolated_database_env, committed_project_id)) command.downgrade(config, "0028_artifact_admission") diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index e5bab647..f253237b 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -414,18 +414,15 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: session.add(Project(id=project_id, name="Checker project", slug=f"checker-{project_id}")) await session.flush() - session.add( - ProjectGuide( - id=guide_id, - project_id=project_id, - version=guide_version, - status="active", - content_markdown="# Checker guide", - approved_by="setup-actor", - effective_at=now, - created_by="setup-actor", - ) + guide = ProjectGuide( + id=guide_id, + project_id=project_id, + version=guide_version, + status="draft", + content_markdown="# Checker guide", + created_by="setup-actor", ) + session.add(guide) await session.flush() session.add( GuideSourceSnapshot( @@ -557,6 +554,10 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: ] ) await session.flush() + guide.status = "active" + guide.approved_by = "setup-actor" + guide.effective_at = now + await session.flush() session.add( WorkstreamTask( id=task_id, From 7529d110d19cc5546cb754c36c19d2541815004a Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:45:41 +0100 Subject: [PATCH 15/27] docs(review): authorize ART fixture ordering --- ...-03P-review-revision-policy-persistence.md | 10 +++++--- backend/tests/test_artifact_admission.py | 23 +++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) 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 251d9f78..411aa369 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 @@ -43,9 +43,13 @@ The following cross-owner files are proof-only exceptions: - `backend/tests/conftest.py`: update only the deterministic public-schema fingerprint for migration 0034. -- `backend/tests/test_artifact_admission.py`: update only ReviewPolicy and - RevisionPolicy fixture construction for schema compatibility; no ART - assertion or behavior may change. +- `backend/tests/test_artifact_admission.py`: update ReviewPolicy and + RevisionPolicy fixture construction for schema compatibility. Where 0034 + requires those immutable policies before publication, the owning helper may + construct the same guide as draft, flush the policies, and then assign its + previously existing active/approval/effective values before Task, Submission, + or CheckerRun setup. The final fixture state and every ART assertion and + behavior must remain unchanged. - `backend/tests/test_tasks.py`: update only policy fixture construction and add immutable-policy regression proof. No Task behavior may change, and the existing stamped review, revision, and compensation-policy isolation diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index f253237b..e5bab647 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -414,15 +414,18 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: session.add(Project(id=project_id, name="Checker project", slug=f"checker-{project_id}")) await session.flush() - guide = ProjectGuide( - id=guide_id, - project_id=project_id, - version=guide_version, - status="draft", - content_markdown="# Checker guide", - created_by="setup-actor", + session.add( + ProjectGuide( + id=guide_id, + project_id=project_id, + version=guide_version, + status="active", + content_markdown="# Checker guide", + approved_by="setup-actor", + effective_at=now, + created_by="setup-actor", + ) ) - session.add(guide) await session.flush() session.add( GuideSourceSnapshot( @@ -554,10 +557,6 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: ] ) await session.flush() - guide.status = "active" - guide.approved_by = "setup-actor" - guide.effective_at = now - await session.flush() session.add( WorkstreamTask( id=task_id, From b9501b31565370306aea168ff4c218a2880e6c0b Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:47:44 +0100 Subject: [PATCH 16/27] test(review): order ART policy fixture before publication --- backend/tests/test_artifact_admission.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index e5bab647..f253237b 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -414,18 +414,15 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: session.add(Project(id=project_id, name="Checker project", slug=f"checker-{project_id}")) await session.flush() - session.add( - ProjectGuide( - id=guide_id, - project_id=project_id, - version=guide_version, - status="active", - content_markdown="# Checker guide", - approved_by="setup-actor", - effective_at=now, - created_by="setup-actor", - ) + guide = ProjectGuide( + id=guide_id, + project_id=project_id, + version=guide_version, + status="draft", + content_markdown="# Checker guide", + created_by="setup-actor", ) + session.add(guide) await session.flush() session.add( GuideSourceSnapshot( @@ -557,6 +554,10 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: ] ) await session.flush() + guide.status = "active" + guide.approved_by = "setup-actor" + guide.effective_at = now + await session.flush() session.add( WorkstreamTask( id=task_id, From ba9c6249c053696dc9f32bfd750a2b9bb787e4f6 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:49:49 +0100 Subject: [PATCH 17/27] docs(review): record complete Backend repair --- .../REVIEW_LOG.md | 14 ++++++++++- ...WS-REV-001-03P-external-review-response.md | 25 ++++++++++++++----- ...WS-REV-001-03P-internal-review-evidence.md | 9 ++++--- .../reviews/WS-REV-001-03P-pr-trust-bundle.md | 19 ++++++++------ 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md index 17c4e127..da56846b 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/REVIEW_LOG.md @@ -1150,4 +1150,16 @@ lifecycle, endpoint, authorization, response, and non-policy behavior remained prohibited. Candidate `b68a1e22b3bf373d15479784d545fcc9b5737f64` removes the five retired inputs, uses canonical finding fields, and supplies the two reviewed durations. Ruff and diff integrity pass, and all required internal -tracks pass on the exact candidate. Fresh GitHub proof remains required. +tracks pass on the exact candidate. + +The completed Backend run then exposed shard 1's six stale 0033 head/rollback +expectations and shard 3's ART fixture policy-after-publication ordering. The +six assertions now name 0034 without changing historical targets. The ART +fixture repair was reverted after an internal scope finding, prospectively +authorized, and reapplied byte-identically: policies flush while the same guide +is draft, then its prior active/approval/effective values are restored before +Task, Submission, or CheckerRun setup. Final code candidate +`b9501b31565370306aea168ff4c218a2880e6c0b` passes the four exact Alembic tests, +the exact ART test, Ruff, diff integrity, and all internal tracks. CodeRabbit +did not perform a review because its limit was reached. Fresh GitHub and actual +CodeRabbit proof remain required. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md index 2a789b11..14fb5a5e 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md @@ -9,6 +9,16 @@ fixture. Candidate `b68a1e22b3bf373d15479784d545fcc9b5737f64` removes those retired inputs, uses `description` and `severity`, and supplies 900-second preference and 1800-second lease durations. +- Shard 1 jobs exposed six stale current-head/atomic-downgrade expectations in + `tests/test_alembic.py`. They now expect sole head + `0034_review_revision_policy`; historical downgrade targets are unchanged. +- Shard 3 job `89267629913` exposed an ART proof fixture that inserted immutable + policies after guide publication. After prospective scope review, the same + guide is constructed draft, the same policies are flushed, and its prior + active/approval/effective values are assigned before Task, Submission, or + CheckerRun setup. Every ART assertion and final fixture fact is unchanged. +- Aggregate `test` failed only because API E2E and shards 1 and 3 failed. + Shards 2 and 4 passed. ## Comments deferred @@ -21,15 +31,18 @@ external check passes. The repair does not start 03A. ## Commands rerun -- Ruff on `backend/scripts/api_contract_e2e.py`: PASS. +- Ruff on the API fixture and both affected test files: PASS. +- Four exact failed Alembic tests: PASS in 341.07 seconds. +- Exact failed ART test: PASS in 6.71 seconds. - `git diff --check`: PASS. - Senior/architecture/reuse, QA/product/test-delta, and security/docs/CI exact-SHA review: PASS. -- Fresh API E2E and full-suite/coverage evidence: pending GitHub Actions after - push. +- Fresh API E2E, all shards, aggregate test, and coverage evidence: pending + GitHub Actions after push. ## Remaining risks -No CodeRabbit finding is open. The first CodeRabbit status passed while noting -service rate limiting, so the final pushed head still requires a fresh status. -No lifecycle or authorization assertion was changed to obtain the repair. +CodeRabbit did not perform a review: its green status explicitly reported that +the review limit was reached and it could not start. An actual current-head +CodeRabbit review remains required. No lifecycle, authorization, or ART +assertion was changed to obtain the repairs. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 8028d4ce..77e12eb8 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -3,7 +3,8 @@ ## Candidate - Trusted base: `bcf1292e1a591e3e84bf8ee212ee7191d80741fa` -- Reviewed code candidate: `35531df254c6b25726d666a5e89eda997b97d792` +- Final reviewed code candidate: `b9501b31565370306aea168ff4c218a2880e6c0b` +- Pre-external-repair implementation candidate: `35531df254c6b25726d666a5e89eda997b97d792` - Signed start: workflow run `30014647556` - Risk: L1 / P2 - Boundary: ReviewPolicy and RevisionPolicy persistence only @@ -14,11 +15,11 @@ Valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `b68a1e22b3bf373d15479784d545fcc9b5737f64` +Reviewed code SHA: `b9501b31565370306aea168ff4c218a2880e6c0b` -Reviewed at: 2026-07-23T16:32:45Z +Reviewed at: 2026-07-23T16:48:45Z -Reviewer run IDs: /root/plan_arch_review@b68a1e22b3bf373d15479784d545fcc9b5737f64; /root/qa_product_review@b68a1e22b3bf373d15479784d545fcc9b5737f64; /root/security_docs_ci_review@b68a1e22b3bf373d15479784d545fcc9b5737f64 +Reviewer run IDs: /root/plan_arch_review@b9501b31565370306aea168ff4c218a2880e6c0b; /root/qa_product_review@b9501b31565370306aea168ff4c218a2880e6c0b; /root/security_docs_ci_review@b9501b31565370306aea168ff4c218a2880e6c0b ## Reviewer results diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md index 5150c8d7..f1980d81 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-pr-trust-bundle.md @@ -73,16 +73,21 @@ coverage and prove at least 90 percent for the materially changed subsystem. Senior engineering, QA/test, security/auth, product/ops, architecture, docs, reuse/dedup, test delta, CI integrity, and circuit breaker all PASS on code -candidate `35531df254c6b25726d666a5e89eda997b97d792` after three repair rounds. +candidate `b9501b31565370306aea168ff4c218a2880e6c0b`. The earlier +`35531df254c6b25726d666a5e89eda997b97d792` candidate predates external CI +fixture repairs. ## External review -PR #195's first run passed preflight and Agent Gates. CodeRabbit returned pass -with no actionable comment while reporting service rate limiting. API E2E found -that its existing Project Guide fixture still sent retired policy fields. The -prospectively reviewed repair changes only that fixture to the canonical 03P -schema; exact-SHA internal review and Ruff pass. Fresh current-head GitHub -Actions and CodeRabbit checks remain required before merge. +PR #195's first run passed preflight, Agent Gates, and shards 2 and 4. It failed +API E2E, shards 1 and 3, and therefore the aggregate test gate. Repairs align +the API policy request fixture, six migration-head expectations, and one ART +fixture's policy-before-publication ordering without changing assertions or +product behavior. Focused failing tests, Ruff, diff integrity, and exact-SHA +internal review pass. CodeRabbit's green context explicitly said it could not +start a review because its review limit was reached; it is not review evidence. +Fresh current-head GitHub Actions and an actual CodeRabbit review remain +required before merge. ## Remaining risks and follow-up work From 6cf3ad4c334c771b95b4450ecd8acdb3bfac62e6 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 17:52:41 +0100 Subject: [PATCH 18/27] docs(review): rebind final repair evidence --- .../reviews/WS-REV-001-03P-internal-review-evidence.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 77e12eb8..01724a0e 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -15,11 +15,11 @@ Valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `b9501b31565370306aea168ff4c218a2880e6c0b` +Reviewed code SHA: `ba9c6249c053696dc9f32bfd750a2b9bb787e4f6` -Reviewed at: 2026-07-23T16:48:45Z +Reviewed at: 2026-07-23T16:51:33Z -Reviewer run IDs: /root/plan_arch_review@b9501b31565370306aea168ff4c218a2880e6c0b; /root/qa_product_review@b9501b31565370306aea168ff4c218a2880e6c0b; /root/security_docs_ci_review@b9501b31565370306aea168ff4c218a2880e6c0b +Reviewer run IDs: /root/plan_arch_review@ba9c6249c053696dc9f32bfd750a2b9bb787e4f6; /root/qa_product_review@ba9c6249c053696dc9f32bfd750a2b9bb787e4f6; /root/security_docs_ci_review@ba9c6249c053696dc9f32bfd750a2b9bb787e4f6 ## Reviewer results From bf322587f7fc967cda088291c8aa27ac2c5bee59 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:17:17 +0100 Subject: [PATCH 19/27] docs(rev): authorize policy truncate guards --- ...V-001-03P-review-revision-policy-persistence.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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 411aa369..dcdd968b 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 @@ -4,9 +4,11 @@ Active through signed `Loop Memory Explicit Event` run `30014647556` on exact trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Implementation candidate -`35531df254c6b25726d666a5e89eda997b97d792` passed every required internal -review track and focused local gate. Publication remains gated on final evidence -review and current-head GitHub Actions, CodeRabbit, and human review. +`b9501b31565370306aea168ff4c218a2880e6c0b` is the reviewed pre-CodeRabbit-repair +candidate that passed every required internal review track and focused local gate. +`35531df254c6b25726d666a5e89eda997b97d792` is historical pre-external-repair +authorization context only. Publication remains gated on final evidence review +and current-head GitHub Actions, CodeRabbit, and human review. ## Goal @@ -139,6 +141,12 @@ The following cross-owner files are proof-only exceptions: guide remains an unactivated draft (`status='draft'`, `effective_at is null`, and `superseded_at is null`). Delete is rejected before dereferencing `NEW` and therefore never waits on guide state. +- Statement-level `BEFORE TRUNCATE` triggers + `trg_review_policies_reject_truncate` and + `trg_revision_policies_reject_truncate` use those table-typed functions to + reject bulk erasure before dereferencing `NEW`; downgrade removes both + truncate triggers before dropping their functions. Direct-SQL proof must + exercise the rejection independently for each policy table. - Policy `id`, `project_id`, `guide_version`, and `created_at` are immutable from insert. Draft replacement updates the existing row only and may change only canonical policy fields plus configuring provenance. The trigger overwrites From fe4b1447a9dc60d192308ebb3f0ed68c8832141e Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:17:42 +0100 Subject: [PATCH 20/27] fix(rev): guard policy truncation --- .../STATUS.md | 10 +++++---- ...-03P-review-revision-policy-persistence.md | 8 ++++--- ...WS-REV-001-03P-external-review-response.md | 15 ++++++++++--- ...WS-REV-001-03P-internal-review-evidence.md | 2 +- ...0034_review_revision_policy_persistence.py | 22 +++++++++++++++++++ backend/tests/test_alembic.py | 3 +++ 6 files changed, 49 insertions(+), 11 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md index 13e45729..2c310e6e 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md @@ -4,10 +4,12 @@ `WS-REV-001-03P` is active through signed `Loop Memory Explicit Event` run `30014647556` on exact trusted main -`bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Implementation candidate -`35531df254c6b25726d666a5e89eda997b97d792` persists only REV-owned immutable -ReviewPolicy and RevisionPolicy facts and has passed every required internal -review track plus focused local evidence. GitHub Actions full-suite/coverage, +`bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Reviewed pre-CodeRabbit-repair +implementation candidate `b9501b31565370306aea168ff4c218a2880e6c0b` persists +only REV-owned immutable ReviewPolicy and RevisionPolicy facts and passed every +required internal review track plus focused local evidence. Candidate +`35531df254c6b25726d666a5e89eda997b97d792` is retained only as historical +pre-external-repair authorization context. GitHub Actions full-suite/coverage, CodeRabbit, and human review remain before merge. The chunk does not implement Project Guide ownership, Task transitions, 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 dcdd968b..5fcdb914 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 @@ -3,9 +3,11 @@ ## Status Active through signed `Loop Memory Explicit Event` run `30014647556` on exact -trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Implementation candidate -`b9501b31565370306aea168ff4c218a2880e6c0b` is the reviewed pre-CodeRabbit-repair -candidate that passed every required internal review track and focused local gate. +trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Superseded history SHA +`94702ae492341402b725ffd6751f8f81ac68100b` +records the content-equivalent CodeRabbit-repair tree that passed every required +internal review track and focused local gate; canonical exact-SHA binding lives +in the internal review evidence after the prospective commit sequence is final. `35531df254c6b25726d666a5e89eda997b97d792` is historical pre-external-repair authorization context only. Publication remains gated on final evidence review and current-head GitHub Actions, CodeRabbit, and human review. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md index 14fb5a5e..d6d184e4 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md @@ -19,6 +19,12 @@ CheckerRun setup. Every ART assertion and final fixture fact is unchanged. - Aggregate `test` failed only because API E2E and shards 1 and 3 failed. Shards 2 and 4 passed. +- CodeRabbit run `9747dead-4c6c-4474-a989-4035a1292ec3` found three valid + current-head issues. The evidence and status artifacts now bind their + reviewed and post-repair candidates consistently, and migration 0034 now + rejects `TRUNCATE` on both immutable policy tables with statement-level + triggers that downgrade removes explicitly. The migration regression test + exercises both rejection paths. ## Comments deferred @@ -39,10 +45,13 @@ external check passes. The repair does not start 03A. exact-SHA review: PASS. - Fresh API E2E, all shards, aggregate test, and coverage evidence: pending GitHub Actions after push. +- Ruff and `git diff --check` for the CodeRabbit repair: PASS. The exact + database-backed migration test could not start locally because + `WORKSTREAM_TEST_DATABASE_URL` is unset; it remains required in GitHub + Actions rather than substituting a weaker local database. ## Remaining risks -CodeRabbit did not perform a review: its green status explicitly reported that -the review limit was reached and it could not start. An actual current-head -CodeRabbit review remains required. No lifecycle, authorization, or ART +The CodeRabbit repair requires current-head GitHub Actions and incremental +CodeRabbit confirmation after push. No lifecycle, authorization, or ART assertion was changed to obtain the repairs. diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 01724a0e..85987e4a 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -3,7 +3,7 @@ ## Candidate - Trusted base: `bcf1292e1a591e3e84bf8ee212ee7191d80741fa` -- Final reviewed code candidate: `b9501b31565370306aea168ff4c218a2880e6c0b` +- Final reviewed code candidate: `ba9c6249c053696dc9f32bfd750a2b9bb787e4f6` - Pre-external-repair implementation candidate: `35531df254c6b25726d666a5e89eda997b97d792` - Signed start: workflow run `30014647556` - Risk: L1 / P2 diff --git a/backend/alembic/versions/0034_review_revision_policy_persistence.py b/backend/alembic/versions/0034_review_revision_policy_persistence.py index 498b2107..b63e1ae9 100644 --- a/backend/alembic/versions/0034_review_revision_policy_persistence.py +++ b/backend/alembic/versions/0034_review_revision_policy_persistence.py @@ -169,6 +169,10 @@ def _create_write_guards() -> None: guide_effective_at timestamptz; guide_superseded_at timestamptz; begin + if tg_op = 'TRUNCATE' then + raise exception 'review and revision policies cannot be truncated' + using errcode = '23514'; + end if; if tg_op = 'DELETE' then raise exception 'review and revision policies cannot be deleted' using errcode = '23514'; @@ -277,6 +281,20 @@ def _create_write_guards() -> None: "guard_revision_policy_write()" ) ) + op.execute( + sa.text( + "create trigger trg_review_policies_reject_truncate before truncate " + "on review_policies for each statement execute function " + "guard_review_policy_write()" + ) + ) + op.execute( + sa.text( + "create trigger trg_revision_policies_reject_truncate before truncate " + "on revision_policies for each statement execute function " + "guard_revision_policy_write()" + ) + ) def upgrade() -> None: @@ -314,6 +332,10 @@ def downgrade() -> None: """Restore 0033 only when every policy is an untouched migrated legacy row.""" _lock_policy_context() _require_lossless_downgrade() + op.execute( + sa.text("drop trigger trg_revision_policies_reject_truncate on revision_policies") + ) + op.execute(sa.text("drop trigger trg_review_policies_reject_truncate on review_policies")) op.execute(sa.text("drop trigger trg_revision_policies_guard_write on revision_policies")) op.execute(sa.text("drop trigger trg_review_policies_guard_write on review_policies")) op.execute(sa.text("drop function guard_revision_policy_write()")) diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 3da2f588..894fc438 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -3039,6 +3039,9 @@ async def row(table: str, row_id: str) -> dict: asyncio.run( execute("delete from revision_policies where id=:id", {"id": revision_id}) ) + for table in ("review_policies", "revision_policies"): + with pytest.raises(IntegrityError, match="cannot be truncated"): + asyncio.run(execute(f"truncate table {table}")) finally: command.upgrade(config, "head") From 0ce636cb7362d6a836c8809b76cd3b445e74a5aa Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:24:57 +0100 Subject: [PATCH 21/27] docs(rev): bind truncate repair evidence --- .../WS-REV-001-review-revision-lifecycle/STATUS.md | 10 +++++----- .../reviews/WS-REV-001-03P-internal-review-evidence.md | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md index 2c310e6e..49c3bf20 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md @@ -4,13 +4,13 @@ `WS-REV-001-03P` is active through signed `Loop Memory Explicit Event` run `30014647556` on exact trusted main -`bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Reviewed pre-CodeRabbit-repair -implementation candidate `b9501b31565370306aea168ff4c218a2880e6c0b` persists -only REV-owned immutable ReviewPolicy and RevisionPolicy facts and passed every -required internal review track plus focused local evidence. Candidate +`bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Reviewed CodeRabbit-repair +implementation candidate `fe4b1447a9dc60d192308ebb3f0ed68c8832141e` +persists only REV-owned immutable ReviewPolicy and RevisionPolicy facts and +passed every required internal review track plus focused local evidence. Candidate `35531df254c6b25726d666a5e89eda997b97d792` is retained only as historical pre-external-repair authorization context. GitHub Actions full-suite/coverage, -CodeRabbit, and human review remain before merge. +incremental CodeRabbit confirmation, and human review remain before merge. The chunk does not implement Project Guide ownership, Task transitions, Submission or artifact behavior, review queues or leases, Review decisions, diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 85987e4a..0d02bfcf 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -3,7 +3,7 @@ ## Candidate - Trusted base: `bcf1292e1a591e3e84bf8ee212ee7191d80741fa` -- Final reviewed code candidate: `ba9c6249c053696dc9f32bfd750a2b9bb787e4f6` +- Final reviewed code candidate: `fe4b1447a9dc60d192308ebb3f0ed68c8832141e` - Pre-external-repair implementation candidate: `35531df254c6b25726d666a5e89eda997b97d792` - Signed start: workflow run `30014647556` - Risk: L1 / P2 @@ -15,11 +15,11 @@ Valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `ba9c6249c053696dc9f32bfd750a2b9bb787e4f6` +Reviewed code SHA: `fe4b1447a9dc60d192308ebb3f0ed68c8832141e` -Reviewed at: 2026-07-23T16:51:33Z +Reviewed at: 2026-07-23T17:19:00Z -Reviewer run IDs: /root/plan_arch_review@ba9c6249c053696dc9f32bfd750a2b9bb787e4f6; /root/qa_product_review@ba9c6249c053696dc9f32bfd750a2b9bb787e4f6; /root/security_docs_ci_review@ba9c6249c053696dc9f32bfd750a2b9bb787e4f6 +Reviewer run IDs: /root/plan_arch_review@fe4b1447a9dc60d192308ebb3f0ed68c8832141e; /root/qa_product_review@fe4b1447a9dc60d192308ebb3f0ed68c8832141e; /root/security_docs_ci_review@fe4b1447a9dc60d192308ebb3f0ed68c8832141e ## Reviewer results From 50197bf812605824585c13efd3bbe8cbed19834c Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:29:32 +0100 Subject: [PATCH 22/27] test(rev): bind truncate guard schema fingerprint --- .../reviews/WS-REV-001-03P-external-review-response.md | 5 +++++ backend/tests/conftest.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md index d6d184e4..832327e1 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md @@ -25,6 +25,11 @@ rejects `TRUNCATE` on both immutable policy tables with statement-level triggers that downgrade removes explicitly. The migration regression test exercises both rejection paths. +- Backend run `30029324884`, shard 4, rejected the intentionally changed public + schema fingerprint after the two authorized trigger objects were added. The + proof-only `tests/conftest.py` constant now binds the observed deterministic + fingerprint `756d2c281c873d5b61e96d89e1f855d8e63fc8ed70380bf231c0325d437d7fe7`; + no reset behavior or table ownership changed. ## Comments deferred diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index eb26c735..b08367b8 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -22,7 +22,7 @@ DDL_LOCK_DIRECTORY = Path("/tmp") EXPECTED_PUBLIC_SCHEMA_SHA256 = ( - "d50e9eb30477e00a27b551b4c7714a020c605b67799c8d8e439287d0a6e512a9" + "756d2c281c873d5b61e96d89e1f855d8e63fc8ed70380bf231c0325d437d7fe7" ) PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", From 5ef77bbf0a97c72ee1063e08204f30c02bb93ad5 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:31:09 +0100 Subject: [PATCH 23/27] docs(rev): authorize guarded policy test reset --- .../WS-REV-001-03P-review-revision-policy-persistence.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 5fcdb914..eb06bac4 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 @@ -46,7 +46,11 @@ docs/template_project_guide.md The following cross-owner files are proof-only exceptions: - `backend/tests/conftest.py`: update only the deterministic public-schema - fingerprint for migration 0034. + fingerprint for migration 0034 and register `review_policies` plus + `revision_policies` in the existing `TRUNCATE_GUARDED_TABLES` reset path so + isolated test cleanup can temporarily disable only their user triggers before + truncation and restore them immediately afterward. Do not weaken the schema + fingerprint check or reset ownership checks. - `backend/tests/test_artifact_admission.py`: update ReviewPolicy and RevisionPolicy fixture construction for schema compatibility. Where 0034 requires those immutable policies before publication, the owning helper may From 58a7335dcc754c21779ede5656929775476bbf1f Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:32:13 +0100 Subject: [PATCH 24/27] test(rev): reset guarded policy tables --- .../reviews/WS-REV-001-03P-external-review-response.md | 7 +++++-- backend/tests/conftest.py | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md index 832327e1..4f9b71bc 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md @@ -26,10 +26,13 @@ triggers that downgrade removes explicitly. The migration regression test exercises both rejection paths. - Backend run `30029324884`, shard 4, rejected the intentionally changed public - schema fingerprint after the two authorized trigger objects were added. The + schema fingerprint after the authorized truncate triggers and their reused + guard-function definitions changed. The proof-only `tests/conftest.py` constant now binds the observed deterministic fingerprint `756d2c281c873d5b61e96d89e1f855d8e63fc8ed70380bf231c0325d437d7fe7`; - no reset behavior or table ownership changed. + both policy tables use the existing guarded-reset path so test cleanup can + temporarily disable and immediately restore their user triggers. No reset + ownership check changed. ## Comments deferred diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index b08367b8..e0a0672a 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -84,6 +84,8 @@ "outbox_events", "project_role_grants", "project_role_qualification_snapshots", + "review_policies", + "revision_policies", ) TestDatabaseReset = Callable[..., Awaitable[None]] DatabaseLock = Callable[[], AbstractContextManager[None]] From 4e05f167de812ec6c9c35062379ff88e5bee2e40 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:34:03 +0100 Subject: [PATCH 25/27] docs(rev): bind guarded reset evidence --- .../WS-REV-001-review-revision-lifecycle/STATUS.md | 2 +- .../reviews/WS-REV-001-03P-internal-review-evidence.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md index 49c3bf20..72c08adf 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md @@ -5,7 +5,7 @@ `WS-REV-001-03P` is active through signed `Loop Memory Explicit Event` run `30014647556` on exact trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Reviewed CodeRabbit-repair -implementation candidate `fe4b1447a9dc60d192308ebb3f0ed68c8832141e` +implementation candidate `58a7335dcc754c21779ede5656929775476bbf1f` persists only REV-owned immutable ReviewPolicy and RevisionPolicy facts and passed every required internal review track plus focused local evidence. Candidate `35531df254c6b25726d666a5e89eda997b97d792` is retained only as historical diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 0d02bfcf..61f5cdf2 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -3,7 +3,7 @@ ## Candidate - Trusted base: `bcf1292e1a591e3e84bf8ee212ee7191d80741fa` -- Final reviewed code candidate: `fe4b1447a9dc60d192308ebb3f0ed68c8832141e` +- Final reviewed code candidate: `58a7335dcc754c21779ede5656929775476bbf1f` - Pre-external-repair implementation candidate: `35531df254c6b25726d666a5e89eda997b97d792` - Signed start: workflow run `30014647556` - Risk: L1 / P2 @@ -15,11 +15,11 @@ Valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `fe4b1447a9dc60d192308ebb3f0ed68c8832141e` +Reviewed code SHA: `58a7335dcc754c21779ede5656929775476bbf1f` Reviewed at: 2026-07-23T17:19:00Z -Reviewer run IDs: /root/plan_arch_review@fe4b1447a9dc60d192308ebb3f0ed68c8832141e; /root/qa_product_review@fe4b1447a9dc60d192308ebb3f0ed68c8832141e; /root/security_docs_ci_review@fe4b1447a9dc60d192308ebb3f0ed68c8832141e +Reviewer run IDs: /root/plan_arch_review@58a7335dcc754c21779ede5656929775476bbf1f; /root/qa_product_review@58a7335dcc754c21779ede5656929775476bbf1f; /root/security_docs_ci_review@58a7335dcc754c21779ede5656929775476bbf1f ## Reviewer results From f19b2d798e49e7c3ebb05a19eaa53638c61237a1 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:42:18 +0100 Subject: [PATCH 26/27] test(rev): reach policy truncate guards --- .../reviews/WS-REV-001-03P-external-review-response.md | 5 +++++ backend/tests/test_alembic.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md index 4f9b71bc..91b94f13 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-external-review-response.md @@ -33,6 +33,11 @@ both policy tables use the existing guarded-reset path so test cleanup can temporarily disable and immediately restore their user triggers. No reset ownership check changed. +- Backend run `30029973617`, shard 1, showed PostgreSQL's foreign-key precheck + rejecting the single-table `TRUNCATE` test before the new trigger could fire. + Each independent negative now uses `TRUNCATE ... CASCADE`, which reaches the + intended table's `BEFORE TRUNCATE` guard and still rolls back without erasing + policy or related rows. ## Comments deferred diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 894fc438..b6d2e2d4 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -3041,7 +3041,7 @@ async def row(table: str, row_id: str) -> dict: ) for table in ("review_policies", "revision_policies"): with pytest.raises(IntegrityError, match="cannot be truncated"): - asyncio.run(execute(f"truncate table {table}")) + asyncio.run(execute(f"truncate table {table} cascade")) finally: command.upgrade(config, "head") From f2becbd35bca08ddcd2ba7c36aa9d14b31fdc787 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Thu, 23 Jul 2026 18:44:09 +0100 Subject: [PATCH 27/27] docs(rev): bind truncate test evidence --- .../WS-REV-001-review-revision-lifecycle/STATUS.md | 2 +- .../reviews/WS-REV-001-03P-internal-review-evidence.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md index 72c08adf..36380cdb 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/STATUS.md @@ -5,7 +5,7 @@ `WS-REV-001-03P` is active through signed `Loop Memory Explicit Event` run `30014647556` on exact trusted main `bcf1292e1a591e3e84bf8ee212ee7191d80741fa`. Reviewed CodeRabbit-repair -implementation candidate `58a7335dcc754c21779ede5656929775476bbf1f` +implementation candidate `f19b2d798e49e7c3ebb05a19eaa53638c61237a1` persists only REV-owned immutable ReviewPolicy and RevisionPolicy facts and passed every required internal review track plus focused local evidence. Candidate `35531df254c6b25726d666a5e89eda997b97d792` is retained only as historical diff --git a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md index 61f5cdf2..c7cb16c1 100644 --- a/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/reviews/WS-REV-001-03P-internal-review-evidence.md @@ -3,7 +3,7 @@ ## Candidate - Trusted base: `bcf1292e1a591e3e84bf8ee212ee7191d80741fa` -- Final reviewed code candidate: `58a7335dcc754c21779ede5656929775476bbf1f` +- Final reviewed code candidate: `f19b2d798e49e7c3ebb05a19eaa53638c61237a1` - Pre-external-repair implementation candidate: `35531df254c6b25726d666a5e89eda997b97d792` - Signed start: workflow run `30014647556` - Risk: L1 / P2 @@ -15,11 +15,11 @@ Valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `58a7335dcc754c21779ede5656929775476bbf1f` +Reviewed code SHA: `f19b2d798e49e7c3ebb05a19eaa53638c61237a1` Reviewed at: 2026-07-23T17:19:00Z -Reviewer run IDs: /root/plan_arch_review@58a7335dcc754c21779ede5656929775476bbf1f; /root/qa_product_review@58a7335dcc754c21779ede5656929775476bbf1f; /root/security_docs_ci_review@58a7335dcc754c21779ede5656929775476bbf1f +Reviewer run IDs: /root/plan_arch_review@f19b2d798e49e7c3ebb05a19eaa53638c61237a1; /root/qa_product_review@f19b2d798e49e7c3ebb05a19eaa53638c61237a1; /root/security_docs_ci_review@f19b2d798e49e7c3ebb05a19eaa53638c61237a1 ## Reviewer results