fix(recovery): prove restore catalog index decoys - #223
Conversation
Add the failing contract for caller-owned catalog inspection of a restored PostgreSQL target. Protected main cannot yet prove required package tables, tenant-status indexes, or forced lifecycle RLS. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Add a caller-owned pg_class probe that accepts a restored target only when required package tables, tenant-status indexes, and forced lifecycle RLS are present. The probe does not execute pg_dump or pg_restore and keeps diagnostics content-free. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Add ADR 0018 and operator doctoring so an isolated restore target is checked before production traffic is pointed at it. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Cover malformed rows, duplicate relations, missing tenant-status indexes, and partial RLS flags so the probe stays at 100% branch coverage. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Require btree access method and default key options on the isolated restore catalog probe. Bind the SQL to packaged lifecycle index shapes and execute same-name decoys on the live packaged PostgreSQL image. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
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[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
What to do next
Keep #216 closed. After every required check on f43c87e is terminal-success, merge only this unchanged head. Do not point production traffic at a restored target until inspect_postgres_restore_catalog returns on a connection you already opened.
Do not open a third catalog-acceptance PR. Keep #208, #212, #214, #215, #221, and #224 on their own lanes.
Review
This is the #216 successor. Catalog SQL now requires btree plus default indoption (0 0 0), so a same-name DESC or hash index is incomplete. Unit tests bind the probe to packaged UNIQUE (tenant_scope, endpoint_alias, remote_batch_id) and idx_llm_remote_batch_jobs_tenant_status_observed column order. The container smoke reuses the packaged image, runs the real ANY(%s) query through Psycopg 3, fail-closes same-name decoys (wrong table, swapped keys, non-unique uq_*), then restores the packaged indexes and accepts again.
Local evidence on this exact head:
- 27 focused acceptance/index-semantics tests passed
pg_llm_batch.postgres_restore_acceptance: 65 statements / 22 branches / 0 miss / 0 partial = 100%- interrogate 100%
- ruff clean
No blocking defect. The probe still does not parse a DSN, execute pg_dump/pg_restore, or treat a receipt as restorability. Writer boundary holds: CHANGELOG/README stay with #212/#214.
Residual (not a merge blocker)
The live smoke does not create a DESC or hash same-name decoy. Those shapes are rejected by the same query the smoke already executes (indoption / pg_am). Do not open a successor just to add those two decoys.
Opclass and collation remain unauthenticated. Treat that as a later catalog-hardening slice, not this landing.
Checks
Required GitHub checks were still queued at review time. Re-verify they are terminal-success on f43c87e before merge. CodeRabbit CLI agent auth timed out in this environment; that is not a merge blocker.
This automation cannot merge.
Sent by Cursor Automation: Fix Issues
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Reviewed exact head f43c87e74f964695c934013e6399928b00273e7a against current protected main@d2f1e32271910a6db98a0757d67194ddadca4566. The catalog probe stays on a caller-owned connection, bounds/validates returned rows with exact primitive types, and authenticates the two lifecycle indexes to the intended table with key order, uniqueness/constraint semantics, validity/readiness, plain-index shape, btree access method, and default key options. The live-smoke path exercises the real parameterized ANY(%s) query and rejects same-name wrong-table, swapped-key, and non-unique decoys before restoring the packaged indexes. No current review threads exist and I found no source-level must-fix in this bounded catalog slice. Opclass/collation remain explicit residual hardening rather than a claim made by this evidence object. Approval applies only to this unchanged head; exact-head required workflows/checks and live ancestry/mergeability still must terminal-success before merge.
2 participants
Successor to #216
Do not merge #216 at
e74afe8. That head addedpg_indexauthentication but only grepped the SQL text. This branch starts from that exact head and proves the query.What to do next
Review this successor, then keep #216 draft. After this exact head is green, merge only this unchanged head. Do not point production traffic at a restored target until the probe returns on a connection you already opened.
Bounded change
indoption(0 0 0) so a same-nameDESCor hash index is incomplete.UNIQUE (tenant_scope, endpoint_alias, remote_batch_id)andidx_llm_remote_batch_jobs_tenant_status_observedcolumn order.ANY(%s)query through Psycopg 3, creates same-name decoys (wrong table, swapped keys, non-uniqueuq_*), requiresinspect_postgres_restore_catalogto fail closed, then restores the packaged indexes and accepts again.Changed paths:
pg_llm_batch/postgres_restore_acceptance.pytests/test_postgres_restore_acceptance_index_semantics.pytests/smoke_restore_catalog_index_semantics.pytests/smoke_restore_catalog_index_semantics.shtests/smoke_postgres_container_logging.shdocs/adr/0018-postgres-restore-catalog-acceptance.mddocs/doctoring/postgres-restore-acceptance.mdWriter boundary
Does not execute
pg_dump/pg_restore, does not open a package-owned DSN in the library probe, and does not touch CHANGELOG/README owned by #212/#214. Do not race #208, #212, #215, or #221.Refs #204. Supersedes #216 for the catalog-acceptance landing.