fix(recovery): rehearse every protected-main migration, not a frozen 0001-0009 list - #298
fix(recovery): rehearse every protected-main migration, not a frozen 0001-0009 list#298seonghobae wants to merge 2 commits into
Conversation
…0001-0009 list The DR rehearsal hardcoded migrations 0001-0009, so migrations 0010-0013 (including the MIGRATION_BACKED job_analysis_snapshot) were never exercised by recovery evidence, while the traceability doc claimed the rehearsal applies the protected-main migration sequence. Enumerate database/migrations in numeric order instead, and bind the behavior with a recovery test that fails closed on a hardcoded/frozen list, a skipped migration, or a reordering hazard. Reseal recovery-manifest.json.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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 (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough복구 리허설이 고정 목록 대신 마이그레이션 디렉터리의 SQL 파일을 숫자 순서로 검색하고 실행합니다. 테스트와 실행 계약은 누락, 중복, 비순차 적용을 검증합니다. 무결성 매니페스트도 갱신되었습니다. Changes복구 마이그레이션 검색 및 검증
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The recovery rehearsal now applies the complete checked-in migration chain in order, with tests guarding against omissions and duplicate prefixes. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
|
Recovery rehearsal audit found one still-valid fail-closed defect in the new migration enumeration. The current exact head Please repair this on the canonical #298 branch rather than weakening the test/gate: capture discovery through a simple assignment whose exit status is checked under Current hosted evidence on this head is otherwise strong: Recovery Rehearsal Quality, Foundation CI, SAST, and Security are SUCCESS. CodeQL remains a separate central settlement-order failure, not evidence against this recovery source. |
|
I moved #298 back to Draft without changing the source head/base. The executable fail-closed finding above is still valid on exact |
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
161468e3d17b3e1964f570ec9bf286ec4045bc8d. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- CodeQL PR/CodeQL compatibility analysis (actions): FAILURE (https://github.com/ContextualWisdomLab/Orgmetra/actions/runs/34550250077/job/103113932533)
- CodeQL PR/CodeQL compatibility analysis (python): FAILURE (https://github.com/ContextualWisdomLab/Orgmetra/actions/runs/34550250077/job/103113932491)
- CodeQL compatibility analysis (actions) check run: failure (https://github.com/ContextualWisdomLab/Orgmetra/actions/runs/34550250077/job/103113932533)
- CodeQL compatibility analysis (python) check run: failure (https://github.com/ContextualWisdomLab/Orgmetra/actions/runs/34550250077/job/103113932491)
- Required Noema Review/noema-review: FAILURE (https://github.com/ContextualWisdomLab/Orgmetra/actions/runs/34550250457/job/103112709717)
- noema-review check run: failure (https://github.com/ContextualWisdomLab/Orgmetra/actions/runs/34550250457/job/103112709717)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: restore-rehearsal-postgres.sh"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: restore-rehearsal-postgres.sh"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: restore-rehearsal.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: restore-rehearsal.md"]
R2 --> V2["docs review"]
Evidence --> S3["Repository file: recovery-manifest.json"]
S3 --> I3["repository behavior"]
I3 --> R3["Review risk: Repository file: recovery-manifest.json"]
R3 --> V3["required checks"]
Evidence --> S4["Test: recovery-rehearsal.test.mjs"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: recovery-rehearsal.test.mjs"]
R4 --> V4["targeted test run"]
OpenCode Review Overview
|
…uous
The rehearsal enumerated migrations with `for migration in $(find ... | sort)`.
Under `set -euo pipefail` a failure inside that command substitution does not
fail the `for` compound command, so a missing or unreadable migration
directory collapsed to an empty iteration and the rehearsal silently skipped
migration application instead of failing closed.
Capture discovery through an assignment whose status is checked, reject an
empty result, validate every discovered basename against the canonical
`^[0-9]{4}_[a-z0-9_]+\.sql$` form, reject duplicate numeric prefixes, and
iterate with `while IFS= read -r` rather than word splitting. Discovery now
runs before any cluster connection or destructive DDL.
The executable recovery contract adds regressions proving that missing,
empty, non-canonical, and duplicate-prefix migration sets fail before the
first `psql -f` application is reached.
|
Recovery discovery fail-closed repair is applied ordinary-forward on this canonical branch. Current exact head: The prior head The repair:
Regression evidence (test-first, RED then GREEN):
The doc copy in This changes recovery executable behavior only; no migration source, workflow trigger, gate, or product semantics changed. Awaiting exact-head Recovery Rehearsal Quality / Foundation / SAST / Security evidence on |
Summary
The DR restore rehearsal (
.github/scripts/restore-rehearsal-postgres.sh) had frozen its applied migrations and could silently omit newer protected-main schema. This PR binds recovery evidence to the checked-in migration set and fails closed when that set cannot be discovered unambiguously.Repair lineage
The first repair replaced the frozen migration list with ordered discovery from
database/migrationsand added executable/documented completeness evidence.Fresh review then found a shell failure-mode gap:
for migration in $(find ... | sort)could collapse discovery failure into an empty iteration under the compound command, so a missing/unreadable migration directory could skip migration application rather than fail closed.9b6eca428c9b500158e3428b9cea461483ee4bcdis the ordinary-forward repair:^[0-9]{4}_[a-z0-9_]+\.sql$basenames;while IFS= read -rinstead of word splitting;psql -fmigration application;recovery-manifest.json.Exact-head authority
develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f9b6eca428c9b500158e3428b9cea461483ee4bcd34570990728: SUCCESS34570990707: SUCCESS34570990699: SUCCESS34570990713: SUCCESS34570990718: FAILURE in the central producer/consumer settlement lane, not a demonstrated recovery-source finding. Python consumer103173279931failed enforcement at2026-09-11T06:47:20Zand completed FAILURE06:47:22Z; Actions consumer103173279976failed enforcement at06:52:35Zand completed FAILURE06:52:36Z; authoritative dispatch103175360364did not start until06:56:50Zand completed SUCCESS06:56:57Z. Fresh canary is handed toContextualWisdomLab/.github#2040in comment5630795137.APPROVEDis claimed.Acceptance
Protected integration requires the current exact head to keep all repository-owned recovery checks GREEN and prove that every canonical migration is discovered exactly once in deterministic order, discovery ambiguity/missing state fails before connection or DDL, the real cross-cluster dump/restore rehearsal succeeds, restored business/audit/outbox invariants remain intact, and all central required governance is admissible.
Keep Draft until those current-head gates and independent review are satisfied. No self/model approval, administrator bypass, gate weakening, no-op retrigger, predecessor-evidence transfer, force-push, destructive rebase, or simple Close is authorized.