Skip to content

feat(recovery): compose isolated logical restore drill - #234

Draft
seonghobae wants to merge 7 commits into
fix/recovery-evidence-weakref-coverage-b84f0c9from
feat/logical-recovery-drill-b84f0c9
Draft

feat(recovery): compose isolated logical restore drill#234
seonghobae wants to merge 7 commits into
fix/recovery-evidence-weakref-coverage-b84f0c9from
feat/logical-recovery-drill-b84f0c9

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

ACTIVE Draft — isolated logical recovery drill

Protected main remains exact b84f0c94154043a3473939c01bb6471de5a129ae. This PR is an explicit child of dependency root #233: base fix/recovery-evidence-weakref-coverage-b84f0c9@5951b7a4d779903b8924abaef2a387cae50b7f54, current exact head feat/logical-recovery-drill-b84f0c9@e85a0c2c8cea6e35864f3b94c99077b8b116814f, Draft and mechanically mergeable. Relative to #233 it changes only:

  • pg_llm_batch/postgres_logical_recovery_drill.py;
  • tests/test_postgres_logical_recovery_drill.py; and
  • tests/test_postgres_logical_recovery_drill_identity_authority.py.

Fresh targeted branch inventory still finds this as the only logical-recovery-drill lane. Fresh formal-review and inline-thread inventories are empty; absence of review is not approval. No source or documentation was written directly to protected main.

Bounded contract

The drill composes the existing recovery-receipt/schema/artifact verification, restore-target isolation, bounded logical restore, archive re-verification, and protected catalog/RLS acceptance seams. Successful output is content-free evidence for that one logical drill only when every invoked primitive's own authority contract is satisfied.

It does not implement physical base backup, WAL archiving/replay, PITR execution, target lifecycle/destruction, application-level readiness, external Fernet/TLS/provider credential recovery, deployment RPO/RTO/HA/DR, or distributed exactly-once semantics. Caller-owned connections, libpq services, trusted-source decisions, credential/key custody, and restore-target teardown remain caller/host authority.

Downstream logical-restore authority blocker — #284 / #254 / #286 / #287

Protected postgres_logical_restore.py still has the original archive/executable authority defects tracked by #254/#286/#287, so this composition cannot become integration-ready merely because its own exact-head workflows are green.

Draft #284 is the sole active logical-restore source writer on feat/postgres-logical-restore-executor-d0a4b30, now exact a8863278b66b2067e272affe93c185c3566ea449. It carries both classes of repair on one non-competing lane:

  • retained archive descriptor authority plus an independently reopened /proc/self/fd/<snapshot> read offset, addressing numeric-FD substitution and shared-open-file-description offset interference; and
  • test-first recovery: retain logical-restore executable inode authority #287 executable authority that opens the selected absolute pg_restore no-follow/nonblocking, accepts only the branch's explicit root-owned regular-executable/no-group-or-other-write/no-set-id policy, and executes the retained inode through /proc/self/fd/<fd> with explicit descriptor inheritance while preserving the caller pathname only as argv[0].

After exact predecessor 8a0aa5b070bc42bb78dc018dc8124f26efab4993, #284's hosted CI exposed a branch-owned test-harness regression rather than a production-policy defect: legacy mocked-child tests used illustrative pg_restore paths that are not installed on hosted runners, while the newly retained executable opener now executes before their mocked subprocess.run(). Current test-only repair a8863278b66b2067e272affe93c185c3566ea449 keeps production unchanged and stubs only those legacy mocked-child tests with a real inert descriptor; the dedicated executable-authority tests remain unstubbed.

Those properties remain ACTIVE-PR, not protected-main truth. Fresh exact-head refetch for unchanged #284@a8863278b66b2067e272affe93c185c3566ea449 now confirms CI 32433492834, Release Acceptance 32433492831, Security Scan 32433492805, and SAST Semgrep 32433492854 are all terminal-success. Fresh #284 formal-review and inline-thread inventories remain empty; absence of review is not approval. These repository-local successes do not integrate #284, remove #233, or transfer across a future head/base.

This drill branch must not race #284, duplicate its source repair, or treat branch-only archive/executable evidence as protected implementation. After the authoritative logical-restore repair reaches protected main, this drill must be reconciled against that exact integrated primitive and composition-level regressions adjusted if its descriptor/executable contract changes.

The #284 executable candidate does not establish package-manager provenance, immutable-host/root-compromise resistance, cross-platform parity, restore validity, PITR, RPO/RTO, HA/DR, CSAP, SOC 2, or certification; this composition cannot inherit claims that the primitive itself does not make.

Existing exact-head validation

For unchanged exact drill head e85a0c2c8cea6e35864f3b94c99077b8b116814f, repository-local workflows previously reached terminal success: CI 32209234284, Security Scan 32209234281, SAST Semgrep 32209234173, and Release Acceptance 32209234290. That evidence proves only this exact head under its then-current dependency tree. It does not prove #284 integrated, transfer to a repaired logical-restore dependency, remove #233, or satisfy final-base/current-last-push review governance.

Dependency / governance boundary

#233 must integrate first. Its repository-local exact-head workflows are terminal-success, but its current formal review remains non-passing. The mutable read-only central .github prerequisite is tracked only in pg-owned issue #244; do not freeze central successor SHAs/run IDs into this PR body, copy central workarounds locally, churn pg heads merely to manufacture review events, self-approve, or weaken gates.

The second independent blocker is the invoked logical-restore primitive: #284 must integrate through its own live quality/security/review governance before this drill can rely on the repaired archive and executable authority contracts.

After both dependencies are protected-main authority, reconcile/retarget this PR to then-current protected main, prove its relative diff and composition semantics again, and reacquire every exact-final-head/current-base CI, supported Python including 3.14, exact 100% owned production statement/branch coverage, public docstrings, security/SAST, package/SBOM/provenance/release, formal-review, unresolved-thread, and then-live governance gate. Merge only with zero valid findings and any qualifying non-author current-last-push approval required by live policy.

Queued, pending, cancelled, skipped-required, absent, neutral, stale, predecessor, status-only, synthetic, author-only, rate-limited, infrastructure-failed, dismissed, or conclusion-null evidence remains non-passing.

Refs #204, #233, #244, #254, #284, #286, #287.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 41f6f6cc-7abe-4e3d-9ebe-278de39f43d8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

Fresh exact-head audit on 1ff6fab0032956d0aca25c815929296a6264ee4f found an evidence-provenance gap that should remain Draft-blocking until either the implementation or the evidence contract is narrowed.

run_postgres_logical_recovery_drill() currently accepts three separately caller-controlled authorities: restore_service_name (used by pg_restore), restore_target_identity (used by the isolation check and copied into restore_system_identifier), and restore_connection (used for post-restore catalog/RLS inspection). verify_postgres_restore_target_isolation() only proves that the supplied names and supplied identity values differ from the live values; it does not prove that the supplied restore_target_identity.system_identifier was observed through restore_connection, nor that restore_connection is the cluster actually reached by restore_service_name.

The success path then emits restore_system_identifier=restore_target_identity.system_identifier. A caller can therefore supply identity A, restore through service B, inspect catalog through connection C, and receive evidence attributing the accepted drill to A as long as the independent lower-layer calls succeed. Existing tests mock these authorities independently and do not exercise this mismatch.

This is an evidence/data-integrity boundary, not a claim that caller-owned connections/services are forbidden. Smallest defensible repair should be test-first: add a RED where the post-restore connection reports a system identifier different from the supplied restore identity and prove the drill fails closed before emitting evidence. Then either (a) bind the emitted identifier to an exact pg_control_system() observation through the same restore connection used for catalog acceptance, with bounded/content-free failure behavior, while retaining an explicit non-guarantee that the libpq service-to-connection binding remains caller/host authority; or (b) if that same-connection observation is intentionally outside package authority, stop emitting/wording restore_system_identifier as observed drill evidence and mark it explicitly caller-asserted. Do not widen this PR into physical PITR or duplicate #233's unrelated aggregate-coverage repair.

Non-destructively retain the bounded logical-recovery drill while including the exact dependency-root weakref coverage regression from #233 head 5951b7a. No recovery authority, protected-main content, or canonical documentation is changed by this branch-only merge.
@seonghobae
seonghobae changed the base branch from main to fix/recovery-evidence-weakref-coverage-b84f0c9 August 19, 2026 02:37
@opencode-agent opencode-agent Bot added area: operations Operability, observability, readiness, SLO, backup, or retention priority: medium Normal-priority or P2 work scope: research Research, statistical validation, or scientific evidence status: draft Draft pull request type: feature New or expanded product capability labels Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: operations Operability, observability, readiness, SLO, backup, or retention priority: medium Normal-priority or P2 work scope: research Research, statistical validation, or scientific evidence status: draft Draft pull request type: feature New or expanded product capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant