feat(composition): persist deployment activation authority - #437
seonghobae wants to merge 258 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough제품 구성 활성화와 복구를 위한 PostgreSQL 레지스트리와 Python API를 추가했습니다. 인가 증거, 소유자 관측, 쓰기 직렬화와 소유권 출처를 검증합니다. 복구 증명 시퀀스에 연결된 경로 스냅샷은 사용 시 현재 상태를 다시 확인합니다. Changes활성화 권한과 복구 흐름
Priority: ⬇️ Low Estimated code review effort: 5 (Critical) | ~90 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Caller
participant AuthorizedPostgresActivationRegistry
participant EvidenceProvider
participant PostgresActivationRegistry
participant PostgreSQL
Caller->>AuthorizedPostgresActivationRegistry: activate 또는 rollback 호출
AuthorizedPostgresActivationRegistry->>EvidenceProvider: 인가 증거 요청
EvidenceProvider-->>AuthorizedPostgresActivationRegistry: ActivationAdmissionEvidence 반환
AuthorizedPostgresActivationRegistry->>PostgresActivationRegistry: 증거 다이제스트와 writer 전달
PostgresActivationRegistry->>PostgreSQL: 배포 행 잠금 및 증거·이벤트 저장
PostgreSQL-->>PostgresActivationRegistry: 저장 결과 반환
PostgresActivationRegistry-->>Caller: 승인된 전환 결과 반환
Merge Risk: ⚪ Minimal · up to This change adds durable activation, rollback, and recovery tracking, plus serving snapshots tied to specific recovery attestations. No blocking defect is established at the current head. Before relying on this for production serving, the owners should confirm two edge cases: reuse of a recovery sequence after a restore, and clock comparisons within the same millisecond. They should also run the PostgreSQL contract suite, which the change's own notes list as still pending. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head=e57195d1cb33c9289093e7bf0885c67f0ca80ff8
red=01096f4753fcf8fd511f0d1240e502012c6c60f8
printf 'HEAD '; git rev-parse HEAD
printf 'requested '; git rev-parse "$head^{commit}"
git show -s --format='%H %P %s' "$head"
git show -s --format='%H %P %s' "$red"
printf '%s\n' '== changed paths =='
git diff --name-status "$red^" "$head" | head -n 90
printf '%s\n' '== exact migration diff =='
git diff --unified=50 "$red^" "$head" -- database/migrations/0025_product_composition_activation_relation_owner_provenance.sql
printf '%s\n' '== 0025 chronology and authority checks =='
git show "$head:database/migrations/0025_product_composition_activation_relation_owner_provenance.sql" | nl -ba | sed -n '1,385p'
printf '%s\n' '== prior attestation schema and recovery SQL =='
git show "$head:database/migrations/0022_product_composition_recovery_attestation.sql" | nl -ba | sed -n '1,182p'
ast-grep outline services/product-composition-api/src/orgmetra_product_composition/activation.py --items all | head -n 95
git show "$head:services/product-composition-api/src/orgmetra_product_composition/activation.py" | nl -ba | sed -n '32,115p;300,415p'
printf '%s\n' '== chronology regression file names =='
git diff --name-only "$red^" "$head" | rg 'recovery|activation|migration|postgres|serving' | head -n 100Length of output: 45241 🏁 Script executed: #!/bin/bash
set -euo pipefail
head=e57195d1cb33c9289093e7bf0885c67f0ca80ff8
src=services/product-composition-api/src/orgmetra_product_composition
testfile=tests/test_product_composition_recovery_attestation_postgres.sh
printf '%s\n' '== regression diff =='
git diff --unified=25 01096f4753fcf8fd511f0d1240e502012c6c60f8^ "$head" -- "$testfile"
printf '%s\n' '== regression focused body =='
git show "$head:$testfile" | nl -ba | sed -n '1,310p'
printf '%s\n' '== serving query and chronology comparison =='
ast-grep outline "$src/serving_snapshot.py" --items all | head -n 85
git show "$head:$src/serving_snapshot.py" | nl -ba | sed -n '30,106p;345,482p'
printf '%s\n' '== recovery completion and post commit wrapper =='
git show "$head:$src/activation.py" | nl -ba | sed -n '393,451p'
git show "$head:$src/activation_runtime_integrity.py" | nl -ba | sed -n '265,345p'
printf '%s\n' '== 0024 binding and 0025 scope =='
git show "$head:database/migrations/0024_product_composition_activation_trigger_function_provenance.sql" | rg -n -C2 'validate_product_composition_recovery_attestation_insert|CREATE TRIGGER product_composition_recovery_attestation_insert_guard|owner|LOCK TABLE' | head -n 90
git diff --name-status 01096f4753fcf8fd511f0d1240e502012c6c60f8^ "$head" -- database/migrationsLength of output: 35439
The repair does not close two previously identified serving gaps. In I inspected source and regression definitions. I did not run Python tests or PostgreSQL contracts. You are interacting with an AI system. |
|
@coderabbitai review Please focus this pass on exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head=f530896ccd3b787e1601b247241621a5674eb4ee
src=services/product-composition-api/src/orgmetra_product_composition
printf 'HEAD '; git rev-parse HEAD
printf 'requested '; git rev-parse "$head^{commit}"
git status --short
git show -s --format='%H %P %s' "$head"
printf '%s\n' '== changed files after preceding chronology fix =='
git diff --name-status e57195d1cb33c9289093e7bf0885c67f0ca80ff8 "$head"
printf '%s\n' '== serving test file discovery =='
fd -t f . services/product-composition-api/tests | rg 'test_activation_route_snapshot_(currentness|recovery_attestation_currentness|deployment_use_boundary)\.py$'
for file in "$src/serving_snapshot.py" services/product-composition-api/tests/test_activation_route_snapshot_currentness.py services/product-composition-api/tests/test_activation_route_snapshot_recovery_attestation_currentness.py services/product-composition-api/tests/test_activation_route_snapshot_deployment_use_boundary.py; do
printf '\n== outline %s ==\n' "$file"; ast-grep outline "$file" --items all | head -n 115
done
printf '%s\n' '== focused diff =='
git diff --unified=25 e57195d1cb33c9289093e7bf0885c67f0ca80ff8 "$head" -- "$src/serving_snapshot.py" services/product-composition-api/tests/test_activation_route_snapshot_currentness.py services/product-composition-api/tests/test_activation_route_snapshot_recovery_attestation_currentness.py services/product-composition-api/tests/test_activation_route_snapshot_deployment_use_boundary.py | head -n 780
printf '%s\n' '== source SQL and request routing =='
nl -ba "$src/serving_snapshot.py" | sed -n '1,120p;265,505p'Length of output: 46597
I found one blocking serving-currentness issue.
The existing tests reject a missing attestation and a different recovery sequence. They do not cover reuse of both the original sequence and evidence digest after restore. Bind each snapshot to a durable recovery issuance identifier that cannot be reused after restore. The serving query must require that identifier. The requested current-head repair is otherwise sound.
You are interacting with an AI system. |
|
Descendant authority update: #438 remains exact |
|
Currentness-owner follow-up finding from the #442 consumer review: That means descendants cannot safely turn expiry into an ordinary 503 by catching Before the deployable host can give a truthful external availability response for expired recovery evidence, this #437 owner should split expected serving-liveness expiry from integrity/authorization failures with a dedicated typed contract (name/shape to be decided here), keep DB wall-clock authority unchanged, and extend the #437 tests before descendants consume it. Do not patch this in #442 by source-copying currentness logic or string-classifying errors. This is an open repair finding, not a reason to close or bypass the stack. |
1 participant
Scope
Advances #435 as the durable deployment activation/rollback/recovery and serving-currentness slice, stacked directly on #436 exact
30d89fa8f4ba95d7ddb84dde8e3b7e5faebf0343. Source head remains8a67b5cef24332619b3a4466e3787a851c299215, open · Draft · mergeable=true, ordinary-forward 258 ahead / 0 behind that exact parent. Orgmetra HR domain truth remains with its bounded-context owners; this slice owns product-composition durable activation/evidence/recovery/currentness only.Migration order remains 0018→0019→0020→0021→0022→0023→0024→0025. The final 0025 validators enforce owner-observation lifetime dominance for activation/recovery evidence. No PostgreSQL GREEN is inferred from source contracts.
recover_active_route_snapshot()remains the startup/reload acquisition path.current_route_ids_for_snapshot()remains the request-time DB linearization boundary for current activation, exact/latest recovery attestation, DB-owned recovery time, clock rewind and evidence expiry. Newer recovery/activation authority invalidates older serving decisions for subsequent requests; no deployment lock is held across owner HTTP I/O.Descendant request/HTTP stack
#438 remains exact
d642eb71c05ae3990349a26facf30593a5703914; #439 remains exact2256112d09a7130cd361264eda8303b12d7ad027; #440 remains exactd8f161af80e2e9bcbd15a51c3d248798b839f4ce; current leaf #442 is exactcd908b65a8047d85e57cb82c0665185c201972b3, 2 ahead / 0 behind #440 with #440 exact as merge base.#438 performs declaration-first concrete-before-template routing and consumes this PR's currentness boundary. #439 binds raw ASGI request-target evidence. #440 uses durable serviceability for dynamic 405
Allow, GET/HEAD route parity and HTTP problem responses. #442 adds one narrow adapter around the two request-routing currentness consumers:ee642f086b8d08a14d1a7f9befef9ee490a1baa7provesActivationConflictErrorfrom this PR's currentness boundary otherwise escapes both a selected route and a resource-specific 405 candidate;cd908b65a8047d85e57cb82c0665185c201972b3translates only that typed superseded-state/conflict signal toCompositionRouteUnavailableError, so the established HTTP layer can emit its existing 503 representation;ActivationAuthorizationErrorremains unwrapped. Descendants are not allowed to infer expiry/integrity/security meaning from exception text or to change this PR's durable currentness rules.This preserves currentness ownership here while distinguishing a normal snapshot supersession from authorization/integrity failures at the consumer boundary. No descendant changes SQL/migration bytes.
Verification boundary
Keep Draft. Canonical acceptance must use one unchanged exact #442
cd908b65a8047d85e57cb82c0665185c201972b3candidate so superseded-state classification, #440 HTTP/current-Allow/GET-HEAD semantics, #439 transport, #438 routing and the inherited #437 suite execute together with exact 100% owned statement/branch/docstring/edge coverage. #311 must execute the complete 0018→0025 inventory on the same candidate, including evidence-lifetime, DB-owned recovery time, optional-route acceptance, hostilesearch_path, 0020/0021 upgrade, recovery serialization and provenance/owner-drift roots. Predecessor workflow/review/status evidence does not transfer.#433 source remains exact
6fc85e4d8bb273adb0d5866d87554067c749179d, ADR 0432 Proposed/Draft. Its source must eventually distinguish durable currentness, typed consumer-side currentness-failure classification, request-time 405 serviceability, GET/HEAD shared resource authority, and the still-missing successful-response owner-dispatch/lifecycle/performance boundary. PR-body currentization alone is not source currentization.This slice is not buyer-ready composition serving. Positive acceptance still requires canonical exact-tree execution, immutable external identity/owner evidence, qualifying independent review, production DB-role/security/fault/recovery evidence, deployable authenticated owner dispatch, and full
edge -> composition -> owner HTTP -> PostgreSQLk6/E2E at p95≤20 ms.No force-push, destructive rebase, self-approval, routine administrator bypass, gate weakening, mutable sibling source, cross-service SQL, feature-local quality workflow, synthetic status, predecessor-GREEN transfer, migration-number theft, premature Ready/merge, or simple Close is authorized.
Refs #435 #436 #434 #433 #432 #438 #439 #440 #442 #260 #311 #100 #51. Keyverse owner dependency: ContextualWisdomLab/keyverse#158.