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 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head=a49701c763ea74219b26a5b5abd60ef5d84685a2
prior=2dc653eb73cf5bd4dbbe29b93b65b7408371112c
printf 'HEAD '; git rev-parse HEAD
printf 'requested '; git rev-parse "$head^{commit}"
git status --short
git show -s --format='%H %P %s' "$head"
git diff --name-status "$prior" "$head" | head -n 100
git diff --name-status "$head^" "$head" | head -n 100
for f in services/product-composition-api/src/orgmetra_product_composition/activation.py services/product-composition-api/src/orgmetra_product_composition/activation_runtime_integrity.py services/product-composition-api/src/orgmetra_product_composition/serving_snapshot.py; do
echo "== outline $f =="; ast-grep outline "$f" --items all | head -n 110
done
printf '== focused diff ==\n'
git diff --unified=25 "$prior" "$head" -- services/product-composition-api/src/orgmetra_product_composition/activation.py services/product-composition-api/src/orgmetra_product_composition/activation_authorization.py services/product-composition-api/src/orgmetra_product_composition/activation_runtime_integrity.py services/product-composition-api/src/orgmetra_product_composition/serving_snapshot.py services/product-composition-api/tests | head -n 1400Length of output: 68723 🏁 Script executed: #!/bin/bash
set -euo pipefail
src=services/product-composition-api/src/orgmetra_product_composition
printf '%s\n' '== deployment identity construction guard and events =='
nl -ba "$src/activation.py" | sed -n '115,255p'
printf '%s\n' '== structural activation and recovery =='
nl -ba "$src/activation.py" | sed -n '260,535p'
printf '%s\n' '== deployment detach =='
nl -ba "$src/activation.py" | sed -n '530,605p'
printf '%s\n' '== wrapper recovery and evidence call =='
nl -ba "$src/activation_runtime_integrity.py" | sed -n '155,345p'
printf '%s\n' '== serving SQL and currentness =='
nl -ba "$src/serving_snapshot.py" | sed -n '32,72p;260,475p'
printf '%s\n' '== focused recovery sequence test changes =='
git diff --unified=8 2dc653eb73cf5bd4dbbe29b93b65b7408371112c a49701c763ea74219b26a5b5abd60ef5d84685a2 -- 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.pyLength of output: 56060 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== exact head =='
git rev-parse HEAD
printf '%s\n' '== recovery attestation migration =='
nl -ba database/migrations/0022_product_composition_recovery_attestation.sql | sed -n '1,175p'
printf '%s\n' '== recovery SQL and sequence retrieval =='
nl -ba services/product-composition-api/src/orgmetra_product_composition/activation.py | sed -n '28,115p'
printf '%s\n' '== relevant currentness and recovery test assertions =='
rg -n -C 3 'recovery_sequence|recovered_at_unix_ms|database wall clock|clock_reads|post.commit|post_commit' services/product-composition-api/tests/test_activation_route_snapshot_recovery_attestation_currentness.py services/product-composition-api/tests/test_authorized_recovery_persistence.py
printf '%s\n' '== database changes since last reviewed head =='
git diff --name-status 2dc653eb73cf5bd4dbbe29b93b65b7408371112c HEAD -- database/migrationsLength of output: 25877
The structural recovery path detaches deployment coordinates before database use. The product recovery wrapper adds no clock or capability check after the commit-capable call returns. Snapshot issuance now checks the returned recovery sequence, but the normal structural path returns a positive sequence from the attestation insert. I inspected source, migration constraints, and focused tests. I did not run Python tests or PostgreSQL contracts. ✅ Action performedReview finished.
|
|
@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 |
Scope
Advances #435 as the durable deployment activation/rollback/recovery and serving-currentness slice, stacked directly on #436 exact
30d89fa8f4ba95d7ddb84dde8e3b7e5faebf0343. Current exact head is8a67b5cef24332619b3a4466e3787a851c299215, open · Draft · mergeable=true, ordinary-forward 258 ahead / 0 behind that exact parent with the parent as merge base. No force-push or destructive rebase is used.Orgmetra keeps HR domain truth. This slice owns product-composition activation/evidence/event/recovery and serving-currentness boundaries only. #436 remains immutable generation/configuration authority; external identity/authorization truth remains with released owners.
Durable authority represented
Migration order remains
0018 -> 0019 -> 0020 -> 0021 -> 0022 -> 0023 -> 0024 -> 0025. No migration number was added or stolen; still-unreleased 0025 remains the final database validator owner for this stack.Required routes require complete operation evidence. Optional routes may have zero observations, but once any operation is observed the route requires complete operation coverage. #434 admission, #436 persistence, #437 application validation and final PostgreSQL validators retain that zero-or-complete contract.
Application evidence already required bundle validity not to outlive any owner-operation observation. Fresh database review found final 0025 activation/recovery validators only required each observation to be fresh at write time. A direct/buggy DB writer could therefore persist longer-lived activation/recovery authority over a shorter-lived owner observation; for recovery this could leave a serving snapshot valid after the owner-operation evidence that justified its route had expired.
PostgreSQL RED
0e4d0b2e481db169108222fba374f89f74ba1ce4adds executable roottests/test_product_composition_activation_evidence_lifetime_postgres.shwith both activation and recovery cases. Repair2311c0deaf8688f26d55da12b03fe11d293ba015makes both final 0025 validators requireevidence_valid_until_unix_ms <= observation.valid_until_unix_ms; activation additionally requiresobservation.observed_at_unix_ms <= wall_clock_unix_ms, matching recovery and preventing a DB-clock rewind from accepting a now-future observation. Source contract127388c326827b097626c4e8ba9f83590837b7a3pins both invariants independently;8a67b5cef24332619b3a4466e3787a851c299215preserves executable mode for the PostgreSQL root. No PostgreSQL GREEN is claimed until canonical execution runs.Recovery and serving authority
route_availability.available_route_ids_for()remains internal evidence-only projection and is not dispatch authority.recover_active_route_snapshot()is the startup/reload acquisition path: authorized recovery re-admits external evidence, rechecks exact active state under the deployment row lock, persists evidence and commits an append-only recovery attestation before issuing a snapshot bound to the committed recovery sequence.current_route_ids_for_snapshot()is the request-routing currentness guard. Its single PostgreSQL statement checks current activation, the exact durable recovery attestation selected by(recovery_sequence, activation_sequence, generation_id, recovery-evidence digest), the deployment's latest recovery sequence, database-ownedrecovered_at, and one materialized PostgreSQL clock sample. Missing attestation, sequence/digest drift, newer recovery, activation supersession, clock rewind or evidence expiry fail closed. The statement is the routing-decision linearization point, not perpetual cache authority.Configuration identity and route projections remain declaration-order independent. Structural activation/rollback/recovery use detached validated deployment coordinates before callback-capable DB use. Request-time serving pins deployment and recovery coordinates across DB I/O. A newer recovery for the same active generation invalidates older snapshots by latest-recovery-sequence comparison.
Descendant request-routing stack
Draft #438 is now exact
d642eb71c05ae3990349a26facf30593a5703914, 7 ahead / 0 behind this exact #437 with #437 as merge base. In addition to concrete-before-template selection, its new REDef991ff...proves unknown declared paths and undeclared methods must be rejected before crossing PostgreSQL currentness; repaird642eb71...moves declared path/method selection ahead of DB availability without weakening #437 currentness revalidation. #438 adds request-selection source/tests only and no migration bytes; canonical package/PostgreSQL evidence must use one unchanged #438 candidate tree.Verification boundary
Keep Draft. RED/fix/source contracts are not canonical execution evidence. Current exact #437 and descendant exact #438 must obtain their own package/PostgreSQL execution; predecessor workflow/review/status evidence is not transferred and COMMENTED reviews are not independent approval.
#260 service/package handoff must point at exact #438 so request-routing tests and inherited #437 suite run together with exact 100% owned statement/branch/docstring/edge coverage. #311 PostgreSQL handoff likewise must point at the same exact #438 candidate and execute migrations 0018→0025, including the evidence-lifetime root, DB-owned recovery time, optional-route acceptance, hostile
search_path, 0020/0021 upgrade, recovery serialization, trigger/function/relation provenance and owner-drift roots.#433 source remains exact
6fc85e4d8bb273adb0d5866d87554067c749179d, ADR 0432 Proposed/Draft. Its source must eventually adopt final admissible heads and distinguish declared configuration, canonical evidence-only projection, canonical recovery-bound snapshot, detached/pinned coordinates, database-owned recovery time, exact clock ordering, owner-observation lifetime dominance, latest-recovery supersession, durable-attestation/current-activation read linearization, request-routing selection-before-availability/currentness, and still-missing deployable HTTP lifecycle/performance evidence. PR-body currentization alone is not source currentization.0025 migration-time provenance does not prove production runtime/migrator least privilege. Migration
0026remains owned by active Employment-absence work and must not be stolen.This slice is not buyer-ready composition serving. Positive acceptance still requires canonical exact-tree execution, immutable external evidence, qualifying independent review, production DB-role/security/fault/recovery evidence, supported restore/failover provenance, a deployable HTTP composition host, and full
edge -> composition -> owner HTTP -> PostgreSQLk6/E2E at the applicable p95 ≤20 ms target.No force-push, destructive rebase, self-approval, routine administrator bypass, gate weakening, mutable sibling source copy, cross-service SQL, feature-local quality workflow, synthetic status, blind/no-op rerun, predecessor-GREEN transfer, migration-number theft, premature Ready/merge, or simple Close is authorized.
Refs #435 #436 #434 #433 #432 #438 #260 #311 #100 #51. Keyverse owner dependency: ContextualWisdomLab/keyverse#158.