Skip to content

feat(recovery): bind physical/WAL/PITR profiles with fail-closed UTF-8 - #220

Closed
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/bc-5ba3b797-62a6-4ec1-80c9-f4001324a7ad-0c35
Closed

feat(recovery): bind physical/WAL/PITR profiles with fail-closed UTF-8#220
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/bc-5ba3b797-62a6-4ec1-80c9-f4001324a7ad-0c35

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Why this exists

PR #217 at 2aaa6d8 binds a caller-owned physical/WAL/PITR profile, but parse_postgres_physical_recovery_profile() encodes the document without catching UnicodeError. Lone-surrogate text such as "\ud800" leaks UnicodeEncodeError. Hosts that catch only PostgresPhysicalRecoveryError miss that path, and the codec diagnostic is not content-free.

The same slice also told operators that a physical+immediate profile may omit WAL. That confuses continuous WAL archive with backup-internal WAL from pg_basebackup -X stream|fetch. Crash-consistent restore still needs the backup's own WAL.

This branch is the landing vehicle for the profile seam. Do not merge #217 at 2aaa6d8.

Operator action

  • Bind backup_method="physical" only for a crash-consistent base backup whose target kind is immediate.
  • Read wal_archive_required=False as "no continuous WAL archive". Still keep backup-internal WAL from pg_basebackup -X stream or -X fetch.
  • Read pitr plus immediate as recovery_target=immediate, not replay-to-end-of-archive.
  • Treat a parse failure as invalid profile JSON. Do not inspect codec exception text.
  • After you bind the profile, run pg_basebackup / restore_command outside this package, then prove schema, tenant/RLS, checkpoint, and lifecycle usability on an isolated target.

Evidence

  • RED: test_parse_rejects_surrogate_text failed with UnicodeEncodeError on "\ud800" before the encode wrap.
  • GREEN: the same input now raises PostgresPhysicalRecoveryError with a content-free JSON diagnostic.
  • test_pitr_immediate_is_consistent_state_not_end_of_archive locks pitr+immediate as a consistent-state stop.
  • Local coverage run --branch --source=pg_llm_batch.postgres_physical_recovery is 100% statement and branch (71 stmts / 22 branches / 0 miss / 0 partial). Physical unit plus documentation tests: 43 passed. Related receipt/schema/backup/physical suite: 106 passed. interrogate 100%. ruff check clean.
  • Docs: docs/doctoring/postgres-physical-pitr-profile.md, docs/adr/0019-postgres-physical-pitr-profile.md, CHANGELOG.

This slice does not execute pg_basebackup, archive WAL, or restore a cluster. It does not claim CSAP, SOC 2, or a complete recovery program. Refs #204. Successor of #217. Does not race #208 backup or #212 restore executors.

Open in Web View Automation 

Record a caller-owned physical or PITR profile without executing
pg_basebackup. Parse wraps lone-surrogate UTF-8 failures as
PostgresPhysicalRecoveryError. wal_archive_required=False means no
continuous archive, not missing backup-internal WAL. pitr+immediate is
recovery_target=immediate, not replay-to-end-of-archive.

Refs #204. Successor of #217.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review (1c2ce7d)

This head repairs the same two #217 defects as #219 (b03d913):

  1. Lone-surrogate profile text now raises PostgresPhysicalRecoveryError instead of UnicodeEncodeError.
  2. Doctoring and tests now treat wal_archive_required=False as no continuous WAL archive, not missing backup-internal WAL. pitr+immediate is a consistent-state stop.

Do not merge this PR. Prefer #219 as the earlier landing vehicle. #219 uses the same five profile files and leaves CHANGELOG/README/ARCHITECTURE to the documentation writers, so it does not race #214. This branch's extra CHANGELOG bullet and longer public docstrings are not a second landing reason.

Do not merge #217 at 2aaa6d8. Keep #208 and #212 as the logical executors. This automation will not self-approve #219 or #220.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants