fix(recovery): fail-closed physical profile parse and WAL wording

[cursor]

Why this exists

#217 at 2aaa6d84 binds a caller-owned physical/WAL/PITR profile, but parse_postgres_physical_recovery_profile leaks UnicodeEncodeError on lone surrogates, and the physical+immediate contract wording says a base backup "may omit WAL". PostgreSQL 18 still requires backup-internal WAL for a crash-consistent restore (pg_basebackup -X stream or -X fetch). wal_archive_required=False only means no continuous archive.

This successor keeps the #217 seam and repairs those two contracts. It does not execute pg_basebackup, archive WAL, or replay a cluster.

Operator action

• Prefer this head over feat(recovery): bind caller-owned physical/WAL/PITR profiles #217 2aaa6d84. Do not merge feat(recovery): bind caller-owned physical/WAL/PITR profiles #217.
• Create an isolated recovery target before you set isolated_target_prepared=True.
• Use backup_method="physical" only with recovery_target_kind="immediate".
• Read wal_archive_required=False as no continuous WAL archive. Keep backup-internal WAL with the base backup.
• Use backup_method="pitr" only when a WAL archive exists; point-in-time kinds time, xid, name, and lsn require that method.
• Treat immediate as end-of-backup consistency, not replay-to-end-of-archive.
• Treat rpo_seconds / rto_seconds as your objectives. The emitted profile always sets package_capability_claim=false.
• After this lands, continue [Resilience/Acquisition] Establish tested PostgreSQL backup and PITR recovery contract #204 with a live pg_basebackup plus WAL replay drill. Keep feat(recovery): execute bounded logical PostgreSQL backups #208 and fix(recovery): accept custom-format restore seek positions #212 as the logical dump/restore executors.

Evidence

• Local coverage run --branch --source=pg_llm_batch.postgres_physical_recovery: 43 passed, 71 stmts / 22 branches / 0 miss / 0 partial = 100%.
• ruff clean on the changed files.
• Docs: docs/doctoring/postgres-physical-pitr-profile.md, docs/adr/0019-postgres-physical-pitr-profile.md.

Writer boundary

Same five files as #217. Does not race #208 backup execution, #212 restore seek, #214 docs overlay, #215 receipt binding, or #216 catalog inspection. CHANGELOG/README/ARCHITECTURE stay with those writers.

This slice does not prove live WAL replay, schema/RLS parity, CSAP, or SOC 2 readiness. Refs #204.

  

[cursor]

Stale comment

Landing vehicle for the #217 repairs

This head (b03d913) is the successor to #217 2aaa6d84. It keeps the evidence-only physical/WAL/PITR profile seam and repairs the two Important defects:

• parse_postgres_physical_recovery_profile now wraps encode("utf-8") in except UnicodeError and raises PostgresPhysicalRecoveryError, matching the receipt parser on main.
• Physical+immediate wording now means no continuous WAL archive, not omitted backup-internal WAL. immediate is end-of-backup consistency, not replay-to-end-of-archive.

Local evidence on this head: 43 tests; coverage run --branch --source=pg_llm_batch.postgres_physical_recovery is 71 stmts / 22 branches / 0 miss / 0 partial = 100%. ruff clean on the changed files.

Do not merge #217. Do not self-approve this PR. Merge only after required checks are terminal-success on unchanged b03d913 and a non-author human approval lands. Keep #208 and #212 as the logical dump/restore executors. Do not start a live #204 pg_basebackup + WAL drill from this branch.

View PR

  

_{Sent by Cursor Automation: Fix Issues}

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[cursor]

Review of #219 at b03d913

This head is the landing vehicle for the #217 repairs. It is cursor-authored, so this is not an approval.

Stated repairs

1. parse_postgres_physical_recovery_profile wraps encode("utf-8") in except UnicodeError and re-raises the typed JSON error with from None, matching the receipt parser. Lone surrogates stay inside that error.
2. wal_archive_required=False now means no continuous WAL archive. It does not waive backup-internal WAL (pg_basebackup -X stream or -X fetch).
3. immediate is end-of-backup consistency. Replay-to-end-of-archive is the absence of a recovery_target* setting, not a recorded kind. pitr+immediate has a success test.

package_capability_claim remains hardcoded false. The seam does not execute pg_basebackup or WAL replay. No Critical or Important fail-open parse path, leaked exception text, type-confusion, or ADR collision with #212/#215/#216 was found on this exact head.

Minor

The documentation test locks backup-internal / continuous-archive wording, but not the immediate end-of-backup clarification. That is not a merge blocker.

Operator next action

Human-review this exact SHA. Merge only after every required check on b03d913 is terminal-success and a non-author approval lands. Do not merge #217 or #220. Do not start a live #204 pg_basebackup / WAL drill on this head. Keep #208 and #212 as the logical dump/restore executors. After this lands, the next unowned recovery slice is the #218 successor (re-inspect current bytes; do not treat exact-type evidence objects as inspection provenance; allocate ADR 0020). Do not open a third physical-profile PR.

  

_{Sent by Cursor Automation: Fix Issues}

[seonghobae]

Reviewed exact head b03d91389afb4b32cd5350f890d9ddc682a7edeb against protected main@d2f1e32271910a6db98a0757d67194ddadca4566. The profile remains evidence-only: it does not execute pg_basebackup, archive WAL, restore a cluster, or claim RPO/RTO capability. Exact-type and bounded JSON parsing fail closed, lone-surrogate encoding is normalized into the package error boundary, PITR requires a WAL archive, point-in-time targets require the PITR method, and package_capability_claim is fixed false. The physical/immediate wording correctly distinguishes backup-internal WAL from a continuous WAL archive. No unresolved review thread or current must-fix finding was found. Approval is for this unchanged head only; merge still requires every then-live exact-head workflow/check to terminal-success and current ancestry/mergeability to remain valid.

[seonghobae]

One current documentation-regression gap remains on exact head b03d91389afb4b32cd5350f890d9ddc682a7edeb against protected main@5267146534a259f85c0985e153f3f6cb1281f58f. The operator document correctly states that PITR immediate means end-of-backup consistency and is not replay-to-end-of-archive, but tests/test_postgres_physical_recovery_documentation.py does not pin that distinction; the existing unresolved inline thread identified the same gap. Because this successor specifically repairs the misleading physical/PITR wording, leaving its defining immediate semantics unguarded permits a future documentation regression while CI stays green. Please add a narrow documentation regression that requires the end-of-backup/non-replay-to-end-of-archive contract in both authoritative documents, then re-run the focused profile/documentation tests and exact 100% owned coverage/docstrings. Do not change the evidence-only runtime semantics or widen this into a live WAL/PITR executor. This review supersedes my earlier approval after re-verifying the unresolved current finding; it is exact-head only and must not transfer after the repair.

[seonghobae]

Reviewed exact current head 8d75bc2dce5909d0967ac961549f379c3b5c0252 against protected main@76e704415651bdef6ceb06efa8db279349bea22e. This head supersedes my prior CHANGES_REQUESTED on b03d91389afb4b32cd5350f890d9ddc682a7edeb: a fresh compare shows the only #219-owned change since that reviewed head is the requested narrow documentation regression, while the remaining head advancement incorporates commits now on protected main. tests/test_postgres_physical_recovery_documentation.py now requires both authoritative documents to retain immediate, end-of-backup, and replay-to-end-of-archive, in addition to the backup-internal-WAL versus continuous-archive contract. The PR's five-file net delta remains evidence-only; it does not execute pg_basebackup, archive/replay WAL, or claim package RPO/RTO capability. The sole visible review thread is resolved. No current source/documentation must-fix remains from the prior review. Approval applies only to this unchanged exact head and does not waive terminal-success exact-head workflows/checks, live ancestry/mergeability, or thread-resolution requirements before merge.

Dismissed as superseded predecessor-head review evidence. This CHANGES_REQUESTED review is bound to b03d913 and requested only the missing immediate/end-of-backup documentation regression. That exact repair was later verified and approved on 8d75bc2; the current head a623bd2 advances from 8d75bc2 only by protected-main integration commits and retains the same five-file PR delta. Current exact-head coverage/opencode checks are terminal-success. Dismissal removes stale negative evidence only. It does not transfer the old approval or satisfy the live ruleset's required qualifying approval of the current last push.